Skip to content

fix: TouchEvent is not available in Safari and Firefox Desktop #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tbouffard merged 3 commits into from
Nov 20, 2022
Merged

fix: TouchEvent is not available in Safari and Firefox Desktop #127

tbouffard merged 3 commits into from
Nov 20, 2022

Conversation

@zhangbiggs
Copy link

@zhangbiggs zhangbiggs commented Oct 19, 2022 •
edited by tbouffard
Loading

Apple M1 Pro
safari: Version 16.0 (17614.1.25.9.10, 17614)
Can't find variable: TouchEvent in safari

closes #76

@tbouffard
Copy link
Member

tbouffard commented Oct 19, 2022 •
edited
Loading

Hi @zhangbihua and thanks for your contributions
Does the error you see look like something similar to the Firefox issue described in #76?
Could you provide the stack trace please?

Thanks in advance
[UPDATE] answer in #76 (comment)

@tbouffard tbouffard mentioned this pull request Oct 19, 2022
Closed
@tbouffard tbouffard added the bug Something isn't working label Oct 19, 2022
tbouffard
tbouffard approved these changes Oct 22, 2022
View reviewed changes
Copy link
Member

@tbouffard tbouffard left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️ Tested on Ubuntu 20 Desktop with Firefox 105.0 and the webkit version provided by playwright v1.27.0 (WebKit 16.0).
I didn't see errors in webkit prior the changes provided by this PR, so testing with webkit on Ubuntu is probably not relevant.

I will let this PR opened for a few days to let others test the changes with Safari and on mobile prior I merge.

mayorovad
mayorovad reviewed Oct 23, 2022
View reviewed changes
@tbouffard tbouffard marked this pull request as draft October 23, 2022 09:38
@tbouffard
Copy link
Member

tbouffard commented Nov 7, 2022

@zhangbihua you recently push new commits that are not related to the fix for Safari and Firefox, could you revert them and tell us what you think about the @mayorovad 's proposal (#127 (comment))?

@zhangbiggs
Copy link
Author

zhangbiggs commented Nov 8, 2022

@tbouffard @mayorovad
This is about fixing other bugs, not about this issue. My negligence has caused your workload. I'm really sorry. I will revert as soon as possible and establish new branch in my repo.

Regarding about the @mayorovad 's proposal, I guess mouseEvent is more generic than TouchEvent in different devices

isMouseEvent(this.evt) || (window.TouchEvent && this.evt instanceof TouchEvent)

May be more effective

@tbouffard
Copy link
Member

tbouffard commented Nov 8, 2022

@zhangbihua thanks for removing extra commit
About your proposal, it's fine to me. The most important thing is to have the window.TouchEvent guard prior invoking TouchEvent

@tbouffard tbouffard changed the title fix: Can't find variable: TouchEvent in safari fix: TouchEvent is not available in Safari and Firefox Desktop Nov 8, 2022
tbouffard and others added 2 commits November 20, 2022 20:51
@tbouffard @mayorovad
add "window.TouchEvent" guard
350a642 
Co-authored-by: Анатолий Майоров <mayorov.ad@outlook.com>
@tbouffard
Merge branch 'development' into development
6cea2ad
@tbouffard tbouffard marked this pull request as ready for review November 20, 2022 19:59
@tbouffard
Copy link
Member

tbouffard commented Nov 20, 2022

As a first implementation, I commited the proposal described in #127 (review). It is known as working and I wanted the fix to be integrated in the upcoming release.
Feel free to open a new Pull Request with another implementation if this one didn't fit well.

@tbouffard tbouffard merged commit 92aef9b into maxGraph:development Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbouffard tbouffard tbouffard approved these changes

+1 more reviewer

@mayorovad mayorovad mayorovad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TouchEvent is not defined error with Firefox Desktop / Can't find variable: TouchEvent error with Safari Desktop

3 participants

@zhangbiggs @tbouffard @mayorovad