Skip to content
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.

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

Fixes touch events for HTML #36557

Merged
merged 1 commit into from Mar 3, 2020
Merged

Fixes touch events for HTML #36557

merged 1 commit into from Mar 3, 2020

Conversation

@Schroedi
Copy link
Contributor

Schroedi commented Feb 26, 2020

Without this patch, the following exception is thrown when the touch
screen is used: TypeError: e.getBoundingClientRect is not a function.
No touch events arrive in the engine.

From my testing, this PR fixes the issue and behaves as expected.

Tested with godot-demo-projects/misc/multitouch_view/, emscripten 1.39.8
and Firefox mobile emulator as well as FF on Android

Without this patch, the following exception is thrown when the touch
screen is used: TypeError: e.getBoundingClientRect is not a function.
No touch events arrive in the engine.

From my testing, this PR fixes the issue and behaves as expected.

Tested with godot-demo-projects/misc/multitouch_view/, emscripten 1.39.8
and Firefox mobile emulator as well as FF on Android
@akien-mga akien-mga requested a review from Faless Feb 26, 2020
@akien-mga akien-mga added this to the 4.0 milestone Feb 26, 2020
@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Feb 27, 2020

For the reference, a workaround for a related issue was documented here: https://www.reddit.com/r/godot/comments/fabf4o/fixed_ios_html5_touch_and_scroll/

@Faless
Faless approved these changes Mar 3, 2020
Copy link
Contributor

Faless left a comment

Looks good, thanks! 🏆

@Faless Faless merged commit 80582ff into godotengine:master Mar 3, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Mar 4, 2020

For the reference, I can't reproduce the issue with 3.2-stable which was built with emscripten 1.38.47-upstream, in chromium 79.0.3945.130 on Linux.

I can however reproduce it in a build of the current 3.2 branch with emscripten 1.39.6, and I confirm that this patch fixes it.

BTW, with Firefox Nightly on Linux I can't get touchscreen buttons to work at all, with or without this PR.

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Mar 4, 2020

To be on the safe side I checked a local build with this PR and emscripten 1.38.47-upstream and it works fine too.

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Mar 4, 2020

Cherry-picked for 3.2.1.

@Faless

This comment has been minimized.

Copy link
Contributor

Faless commented Mar 4, 2020

I think the problem with touchscreenbutton is that we currently don't detect touch fingers release, not sure it's not a regression of some kind, but I think it was never implemented.

@ChronoDK

This comment has been minimized.

Copy link

ChronoDK commented Mar 6, 2020

Just tested 3.2.1 RC2 and touch is working on iOS without any workarounds now. Nice!

@Schroedi Schroedi deleted the Schroedi:fix_html_touch branch Mar 29, 2020
Schroedi added a commit to Schroedi/godot that referenced this pull request Mar 29, 2020
Similar to godotengine#36557

At least in chrome, the following error is printed for each mouse wheel
rotation:
[Intervention] Unable to preventDefault inside passive event listener due to target being treated as passive. See https://www.chromestatus.com/features/6662647093133312

This PR moves the handler to the canvas and thereby fixes the error.

Tested on: Chrome and Firefox (MacOS), Firefox, Chrome(Android), Safari (IPad + MacOS)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.