798821 - Disable touch events on Windows for devices that do not support touch input

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Bug 726615 broke lots of web pages.

Comment 1

[Jim Mathies [:jimm]]

So basically we're saying we won't support touch events on Windows by default? This doesn't seem like the right solution.

I was thinking we could leave this in for a merge or two to see if sites catch on and fix their handling.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I was thinking we could keep touch events enabled on Nightlies, but not on branches.

Do we know how Chrome deals with this situation?

Comment 3

[Jim Mathies [:jimm]]

(In reply to Olli Pettay [:smaug] from comment #2)
> I was thinking we could keep touch events enabled on Nightlies, but not on
> branches.
> 
> Do we know how Chrome deals with this situation?

Based on some simple chrome testing, it looks like they disable by default and flip the pref when they detect a touch input digitizer. We could add something like that pretty easily.

It would mean web sites that improperly use the presence of the handlers as a method of detecting touch input support would remain broken. At least the behavior would be common for two browsers.

Comment 4

[Makoto Kato [:m_kato]]

Most compatibility issue are

- Touch event is used to detect iOS/Android (Bug 798935 for foxnews)
- Touch event is used to detect touch screen-based browser (Bug 798804 for bbc, Bug 797143 for apple, Bug 795861 for slideshare and etc).  Web site UI is changed for touch.
- If there is touch event in DOM, web site doesn't handle mouse event. (Bug 799852, Bug 798746 for NAVER and etc)

Of course, all issue occurs on Chrome w/ touch enabled, too.

Comment 5

[Jim Mathies [:jimm]]

> Of course, all issue occurs on Chrome w/ touch enabled, too.

I've only found this to be the case when chrome is running on a touch capable device. Are you seeing the same Makoto?

Comment 6

[Bundyo]

I can confirm the behavior in Aurora 18a2 - touch events are enabled even though they are not supported by the hardware.

Chrome 22 enables touch events by default on touch supporting Windows 7 and Windows 8 devices (though this wasn't listed in the Changelog (and hard to find in the bug reports) and caught many by surprise). On Desktop, Chrome 22 has touch support disabled.

The issue as Makoto said is that most mobile enabled sites are using this simple feature detection

"ontouchstart" in window

and then attach to touch events only. The problem they try to avoid is that most mobile browsers fire synthetic mouse events after all touch events has passed and if you bind to both, you'll have to deal with additional event handling to avoid false positives.

To be frank, in this situation Microsoft's implementation of PointerEvent is easier to use and works in both platforms without breaking one or the other.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Jimm, are you working on the "detect a touch input digitizer" part, 
or should we just disable the events on Aurora?

Comment 8

[Jim Mathies [:jimm]]

(In reply to Olli Pettay [:smaug] from comment #7)
> Jimm, are you working on the "detect a touch input digitizer" part, 
> or should we just disable the events on Aurora?

Yeah I can work that up.

Comment 9

[Jim Mathies [:jimm]]

(In reply to Bundyo from comment #6)
> and then attach to touch events only. The problem they try to avoid is that
> most mobile browsers fire synthetic mouse events after all touch events has
> passed and if you bind to both, you'll have to deal with additional event
> handling to avoid false positives.

According to the W3C spec this shouldn't happen if content calls preventDefault on the touch event - 

"If the preventDefault method of touchstart or touchmove is called, the user agent should not dispatch any mouse event that would be a consequential result of the the prevented touch event."

> To be frank, in this situation Microsoft's implementation of PointerEvent is
> easier to use and works in both platforms without breaking one or the other.

This spec isn't far enough along for us to implement by default but we are keeping an eye on it.

Comment 10

[Bundyo]

(In reply to Jim Mathies [:jimm] from comment #9)
> According to the W3C spec this shouldn't happen if content calls
> preventDefault on the touch event - 
> 
> "If the preventDefault method of touchstart or touchmove is called, the user
> agent should not dispatch any mouse event that would be a consequential
> result of the the prevented touch event."

Yes, but this means that we should prevent all touch events in order to avoid the mouse ones and no click events will be fired. One problematic use case can be when you are making a framework and you don't know what the user is going to bind to.

Comment 11

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #8)
> (In reply to Olli Pettay [:smaug] from comment #7)
> > Jimm, are you working on the "detect a touch input digitizer" part, 
> > or should we just disable the events on Aurora?
> 
> Yeah I can work that up.

This is a bit trickier than I anticipated due to the early init of dom class info, which makes the one-time pref query in nsDOMTouchEvent. I was hoping I'd have an early enough call down in widget where the touch detection code is, but it looks like I'm going to be stuck with putting a bit of win centric code in the event class. :/

Comment 12

[Jim Mathies [:jimm]]

Attachment: fix v.1

This should be safe since we don't do shared lib builds for Windows anymore, afaik. Need to confirm this and run the patch through try as well.

Comment 13

[Boris Zbarsky [:bzbarsky]]

We really need to finish migrating that stuff to WebIDL.  :(

Comment 14

[Jim Mathies [:jimm]]

Try build run - https://tbpl.mozilla.org/?noignore=0&tree=Try&rev=0995a8e3eca0

I made one very small change to a comment here as well - "On Windows if there's no default or user pref set we check manually based on device support."

Comment 15

[Benjamin Smedberg]

Comment on attachment 671860 [details] [diff] [review]
fix v.1

I don't think this is a good way to do the pref. We should either change dom.w3c_touch_events.enabled to be a 3-state pref (enabled/disabled/autodetect) or we should have a separate bool pref (dom.w3c_touch_events.autodetect).

Is this sytem metric "static", or does it change as you connect and disconnect potential touch devices? Might the value change while Windows is booting up and connecting USB devices? Since we only check the value once, I'm worried that we might end up disabling touch when we didn't mean to.

Comment 16

[Jim Mathies [:jimm]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #15)
> Comment on attachment 671860 [details] [diff] [review]
> fix v.1
> 
> I don't think this is a good way to do the pref. We should either change
> dom.w3c_touch_events.enabled to be a 3-state pref
> (enabled/disabled/autodetect) or we should have a separate bool pref
> (dom.w3c_touch_events.autodetect).

3-state sounds good.


> Is this sytem metric "static", or does it change as you connect and
> disconnect potential touch devices? Might the value change while Windows is
> booting up and connecting USB devices? Since we only check the value once,
> I'm worried that we might end up disabling touch when we didn't mean to.

I'm not sure. I don't think we need to worry about startup issues though. Input device hardware should be fully connected before the browser starts. I suppose if you had some sort of external wacom digitizer it might change while the browser is running, but for the type of hardware we are interested in here it should be static. If this settings does change dynamically for a common hardware use case we have a more wide ranging problem since this value also gets cached by the css rule processor.

Comment 17

[Jim Mathies [:jimm]]

Attachment: 3-state fix v.1

Comment 18

[Jim Mathies [:jimm]]

Comment on attachment 672821 [details] [diff] [review]
3-state fix v.1

try test run:

https://tbpl.mozilla.org/?noignore=0&tree=Try&rev=a8e9ca869438

Comment 19

[Benjamin Smedberg]

Comment on attachment 672821 [details] [diff] [review]
3-state fix v.1

In the NS_ERROR case, please set sPrefValue = false. I think this could probably be a warning not an error, but I don't think I much care.

Comment 20

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/321ee246dc59

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/321ee246dc59

Comment 22

[[:philipp]]

is there a possibility to uplift the fix to firefox 18, since it breaks functionality on quite a few major sites? thanks

Comment 23

[Jim Mathies [:jimm]]

Comment on attachment 672821 [details] [diff] [review]
3-state fix v.1

We should get this uplifted to aurora as well. Feature was originally enabled in bug 726615.

Comment 24

[Jim Mathies [:jimm]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7d89cb78c6df

Comment 25

[juan becerra [:juanb]]

I tested this on Win7 machines with touch and non-touch screen machines. I didn't notice any problems with the non-touch machines I tried using the latest nightly and aurora.

Comment 26

[Ahmad Saleem]

Sorry for bumping this old thread but I edited MDN wiki for Touch events to reflect that touch events now have tri-state preference, I am not pro in this and this is my first attempt to help Mozilla community, so you people will not mind it.
Here is updated wiki link: https://developer.mozilla.org/en-US/docs/DOM/Touch_events

So dev-doc-needed whiteboard is not valid I think anymore.

Comment 27

[Matt Brubeck (:mbrubeck)]

Thanks!