Closed
Bug 1356695
Opened 6 years ago
Closed 6 years ago
Firefox 52 on touch-enabled laptop: touch API present, but no touch events fired
Categories
(Core :: DOM: Events, defect)
Tracking
()
VERIFIED
FIXED
mozilla55
People
(Reporter: redux, Assigned: kats)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
2.36 KB,
patch
|
smaug
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
952 bytes,
patch
|
milan
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36 Steps to reproduce: From version 52, touch events have apparently been reenabled on desktop. However, beyond the API being reported, touch events themselves don't seem to fire. Actual results: Testing on a Windows 10/Surface 3 laptop with Firefox 52.0.2 (32-bit) with default about:config (dom.w3c_touch_events_enabled set to 2), I see that the API is correctly reported as being present (https://patrickhlauke.github.io/touch/tests/touch-feature-detect.html shows all green). When testing actual events, it appears no touch events are being fired (e.g. https://patrickhlauke.github.io/touch/tests/event-listener.html only shows mouse events/click when tapping, https://patrickhlauke.github.io/touch/tracker/multi-touch-tracker-pointer-hud.html only recognises one finger but as a mouse, none of the touch-events-specific demos/tests in https://patrickhlauke.github.io/touch/ work) Expected results: Touch events should fire, multi-touch should work (e.g. in the tracker demo)
Updated•6 years ago
|
Component: Untriaged → DOM: Events
Product: Firefox → Core
Assignee | ||
Comment 1•6 years ago
|
||
Can you go to about:support and check what it says for Multiprocess Windows?
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(redux)
Reporter | ||
Comment 2•6 years ago
|
||
Multiprocessor Windows: 0/1 (Disabled by accessibility tools) I do have NVDA https://www.nvaccess.org/ and aViewer https://www.paciellogroup.com/resources/aviewer/ installed (but not running during my testing)
Flags: needinfo?(redux)
Assignee | ||
Comment 3•6 years ago
|
||
Touch events are only enabled when e10s is enabled, and it seems to be off for you. In releases 51 and earlier touch screens were being detected as accessibility devices and so e10s was off by default. IIRC in 52, on 32-bit builds, we fixed that, so touchscreen devices should have e10s enabled. 53 will extend that support to 64-bit builds. There are also other factors such as actual accessibility tools and add-ons that might cause e10s to be disabled. It's not a great situation but that's how it is. For now if you want to force e10s on to test touch events, you can go to about:config and create a new boolean preference browser.tabs.remote.force-enable, set it to true, and restart the browser.
Comment 4•6 years ago
|
||
Kats, but why do we exposed the interface to the dom if it's disabled? Doesn't that break feature detection?
Flags: needinfo?(bugmail)
Assignee | ||
Comment 5•6 years ago
|
||
That... is a bug, I guess :/
Assignee: nobody → bugmail
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(bugmail)
Reporter | ||
Comment 6•6 years ago
|
||
For what it's worth, recent Chrome versions expose window.TouchEvent on all devices (even when there's no touchscreen present). But yes, they only return true for things like "'ontouchstart' in window", "'ontouchend' in document" and other classic "sniff to see if it's a touch/mobile device" if there's an actual touchscreen (see https://bugs.chromium.org/p/chromium/issues/detail?id=392584 / https://patrickhlauke.github.io/getting-touchy-presentation/#46)
Reporter | ||
Comment 7•6 years ago
|
||
Confirming that creating the new browser.tabs.remote.force-enable == true in about:config made touch events work as expected in my configuration. So this bug (beyond the "why aren't they working out of the box for me") is now mostly about not exposing the API (either none of it, or at least not the "'ontouchstart' in window" type detects) in situations where touch is not in fact working/disabled
Reporter | ||
Comment 8•6 years ago
|
||
(plus of course would be good to get the above info around multiprocessor and having to force it explicitly in certain configurations documented somewhere like MDN)
Assignee | ||
Comment 10•6 years ago
|
||
I wrote a patch to fix this, it resulted in some test failures because we apparently initialize the touch events before the graphics stack is up and running on marionette tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb08876db238a93a5ee524f362b2202bc45e68d5 I have another try push going at https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbbfee930f72c86605ccd85e01b5a8528171e3a2 which should address that problem.
Assignee | ||
Comment 11•6 years ago
|
||
Attachment #8858545 -
Flags: review?(bugs)
Assignee | ||
Comment 12•6 years ago
|
||
Attachment #8858546 -
Flags: review?(milan)
Updated•6 years ago
|
Attachment #8858546 -
Flags: review?(milan) → review+
Comment 13•6 years ago
|
||
[Tracking Requested - why for this release]: We should probably uplift this as far as we can given its a web compat issue being noticed by sites. Maybe too late for 53.0, but maybe it can ride along if there is a point release?
status-firefox52:
--- → wontfix
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox55:
--- → affected
status-firefox-esr52:
--- → affected
tracking-firefox53:
--- → ?
Flags: webcompat?
Assignee | ||
Comment 14•6 years ago
|
||
(In reply to Ben Kelly [not reviewing due to deadline][:bkelly] from comment #13) > We should probably uplift this as far as we can given its a web compat issue > being noticed by sites. Note that it's more of an issue for developers than for sites. The exposure of the DOM touch APIs to web content just means that we support touch events, it doesn't mean that we *need* to send touch events. From the point of view of web content, the current behaviour just means "the device supports touch, but the user isn't using touch" which should be a perfectly valid scenario. For content authors trying to test their websites with touch, it's more of a bug.
Comment 15•6 years ago
|
||
Oh, I was under the impression sites were taking an x-vs-y decision based on if touch events were enabled. Still would be nice to uplift, but maybe not as critical as I thought.
Assignee | ||
Comment 16•6 years ago
|
||
(In reply to Ben Kelly [not reviewing due to deadline][:bkelly] from comment #15) > Oh, I was under the impression sites were taking an x-vs-y decision based on > if touch events were enabled. Some sites do assume touch and mouse are mutually exclusive. But sites that make such an assumption are broken already, so I wouldn't consider this a webcompat issue.
Reporter | ||
Comment 17•6 years ago
|
||
Sites that assume "if touch API present, only listen for touch" are indeed already broken by design on multi-input environments. However, they're even more broken with the current situation in Firefox, because the user can't then get anything to work at all (rather than being at least able to use the touchscreen).
Comment 18•6 years ago
|
||
Comment on attachment 8858545 [details] [diff] [review] Part 1 - Don't expose DOM touch APIs if APZ is disabled ok, this doesn't seem to affect how parent process is handled.
Attachment #8858545 -
Flags: review?(bugs) → review+
Comment 19•6 years ago
|
||
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b0e0a29a5caa Don't expose DOM touch APIs with autodetection if APZ is disabled. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/fd9fa4e180c6 Ensure gfxPrefs is initialized if gfxPlatform::AsyncPanZoomEnabled is called early in startup. r=milan
Comment 20•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b0e0a29a5caa https://hg.mozilla.org/mozilla-central/rev/fd9fa4e180c6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee | ||
Comment 21•6 years ago
|
||
Comment on attachment 8858545 [details] [diff] [review] Part 1 - Don't expose DOM touch APIs if APZ is disabled Approval Request Comment [Feature/Bug causing the regression]: APZ touch support [User impact if declined]: for Windows users with touch devices but e10s disabled (due to addons or accessibility or whatever), the browser will report that touch events are supported, but will actually convert the touch events to mouse events. This can result in content author frustration and worse user-observable behaviour on sites that don't handle multiple input types properly. [Is this code covered by automated tests?]: not particularly [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: yes. on a windows touch-enabled device, disable e10s. Then open the developer tools console on any page and type "window.TouchEvent". If the result comes back as "undefined" then the bug is fixed. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: not very [Why is the change risky/not risky?]: The logic is relatively straightforward but it's part of a bigger code block with many codepaths and branches to handle different platforms and environments. [String changes made/needed]: none
Attachment #8858545 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•6 years ago
|
Attachment #8858546 -
Flags: approval-mozilla-aurora?
Comment 22•6 years ago
|
||
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Updated•6 years ago
|
Flags: qe-verify+
Comment 23•6 years ago
|
||
I verified this fix on Windows 8.1 x64 and Windows 10 x64 on Surface Pro 2 using Firefox Nightly 55.0a1(2017-04-18) and I can confirm the fix.
Updated•6 years ago
|
tracking-firefox53:
? → ---
Comment 24•6 years ago
|
||
Comment on attachment 8858545 [details] [diff] [review] Part 1 - Don't expose DOM touch APIs if APZ is disabled 54 was merged to Beta today.
Attachment #8858545 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Updated•6 years ago
|
Attachment #8858546 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment 25•6 years ago
|
||
Comment on attachment 8858545 [details] [diff] [review] Part 1 - Don't expose DOM touch APIs if APZ is disabled Fix a touch event support issue and was verified. Beta54+.
Attachment #8858545 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•6 years ago
|
Attachment #8858546 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 26•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/6073f26ea1db https://hg.mozilla.org/releases/mozilla-beta/rev/1d567ed51160
Comment 27•6 years ago
|
||
Verified as fixed on Firefox 54 beta 1 on a Microsoft Pro 2 tablet with Win 10 64-bit.
Flags: qe-verify+
Comment 28•6 years ago
|
||
I've added a note about what's going on here to the main touch events page: https://developer.mozilla.org/en-US/docs/Web/API/Touch_events#Firefox_touch_events_and_multiprocess_(e10s) Does this explain the situation adequately to you Patty? Happy to work on this further ;-)
Flags: needinfo?(redux)
Keywords: dev-doc-needed → dev-doc-complete
Reporter | ||
Comment 29•6 years ago
|
||
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #28) > I've added a note about what's going on here to the main touch events page: > > https://developer.mozilla.org/en-US/docs/Web/API/ > Touch_events#Firefox_touch_events_and_multiprocess_(e10s) > > Does this explain the situation adequately to you Patty? Happy to work on > this further ;-) Minor tweak suggestions (you knew that was coming ;) ): - at the end of the first para, perhaps spell it out: "This means that even on a touchscreen enabled desktop/laptop, touch events won't be enabled." - in the second para: "going to about:support and look at" > change look to looking - third para: "for example to test touch events" arguably, it's not so much the testing, but actually getting the touchscreen to work (with touch events). so i'd reword that as "for example to explicitly reenable touch events support".
Flags: needinfo?(redux)
Comment 30•6 years ago
|
||
(In reply to Patrick H. Lauke from comment #29) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #28) > > I've added a note about what's going on here to the main touch events page: > > > > https://developer.mozilla.org/en-US/docs/Web/API/ > > Touch_events#Firefox_touch_events_and_multiprocess_(e10s) > > > > Does this explain the situation adequately to you Patty? Happy to work on > > this further ;-) > > Minor tweak suggestions (you knew that was coming ;) ): > > - at the end of the first para, perhaps spell it out: "This means that even > on a touchscreen enabled desktop/laptop, touch events won't be enabled." > > - in the second para: "going to about:support and look at" > change look to > looking > > - third para: "for example to test touch events" arguably, it's not so much > the testing, but actually getting the touchscreen to work (with touch > events). so i'd reword that as "for example to explicitly reenable touch > events support". The changes look good, so I've made them. Cheers!
You need to log in
before you can comment on or make changes to this bug.
Description
•