Bug 139681

Summary: Touch support is reported even when the device doesn't have a touch screen
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: carlosg, cgarcia, chavarria1991, clopez, darin, graouts, hi, mcatanzaro, mcdado, nekohayo, pochu27, simon.fraser, svillar, webkit-bug-importer, zan
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://people.igalia.com/clopez/checktouch.html
Bug Depends on:    
Bug Blocks: 141466    
Attachments:
Description Flags
Patch
mcatanzaro: review-
Patch mcatanzaro: review+

Description Carlos Alberto Lopez Perez 2014-12-16 09:59:57 PST
Hi,

WebKitGTK+ reports that it has touch capabilities support even when the device where it's running don't has such capabilities. My laptop don't has any touch support and on the following test I get reported that my device has a touch screen. http://people.igalia.com/clopez/checktouch.html

Both with Firefox=34 and Chromium=38 (Xorg) on the same device (my laptop) and on the same test I get reported that I have a normal screen device without touch support.

Ideally, WebKitGTK+ should check if the device where it runs has a touch screen device (not sure how to do this) and then report touch capabilities based on that.

This has caused for example that click events don't work on the JS plot library (Flotr2) <https://github.com/HumbleSoftware/Flotr2/pull/295>. This is a bug on that library since the user can still use a mouse even if her device has touch capabilities.

But I think that we should not report touch capabilities when the device where we run don't has that, otherwise from time to time some pages that assume that we are touch capable are going to break.

More info: https://hacks.mozilla.org/2013/04/detecting-touch-its-the-why-not-the-how/
Comment 1 Carlos Garcia Campos 2014-12-17 06:25:42 PST
The problem is that TOUCH_EVENTS is a compile time option. Both firefox and chromium say I'm using a normal screen in my touch screen
Comment 2 Carlos Alberto Lopez Perez 2014-12-17 07:33:22 PST
(In reply to comment #1)
> Both firefox and chromium say I'm using a normal screen in my touch screen

According to http://caniuse.com/#feat=touch Chromium should support it.

If you open chrome://flags/#touch-events on chromium, there is a "Enable touch events" switch. On my case is set to auto. Maybe if you force it to on (instead of auto) works.

The code they use for detecting if you have a touch screen seems to be there: https://chromium.googlesource.com/chromium/chromium/+/trunk/ui/events/x/touch_factory_x11.cc (Line 81)
Comment 3 Carlos Garcia Campos 2014-12-17 08:37:07 PST
It's automatic here as well
Comment 4 Carlos Garcia Campos 2014-12-17 10:27:26 PST
We can make touch also a runtime feature, it's indeed listed in RuntimeEnabledFeatures, but ot actually used. However, I'm not sure it's possible to use "ontouchstart" in window to check if touch is enabled. That could be used to check if wk has been build with touch events enabled, but not if the feature is enabled at runtime. I don't think it's possible to not include attributes depending on runtime features. In case of touch being disabled at runtime, you could check if window.ontouchstart is undefined, for example, or if document.createEvent('TouchEvent') raises an exception.
Comment 5 Carlos Alberto Lopez Perez 2014-12-17 13:24:14 PST
I guess we need something like what was implemented on https://bugs.webkit.org/show_bug.cgi?id=30240 for V8, but for JSC.
Comment 6 Carlos Garcia Campos 2014-12-18 09:31:46 PST
Created attachment 243496 [details]
Patch

This disables touch events at run time, but "ontouchstart" in window would still return true. I still think the right check should be something like "ontouchstart" in window && typeof window.ontouchstart != 'undefined' to check both that wk is compiled with touch events enabled and that there's a touchscreen connected.
Comment 7 Carlos Garcia Campos 2015-01-08 08:32:27 PST
Darin, what do you think about this approach? Do you know whether it's possible/desirable to make "ontouchstart" in window return false?
Comment 8 Darin Adler 2015-01-08 08:59:32 PST
It really bothers me that website developers check for the presence of the "ontouchstart" property in the window object as the way to detect "touch capabilities". The whole point of that article is that it’s a bad idea. It’s certainly a hack, not a well considered technique. These "on<xxx>" properties are conveniences that allow you to set up an event listener without calling the addEventListener function. But the presence of such a convenience doesn’t tell you what events the webpage will receive! I don’t think the fact that a web engine allows you to set up a handler for the "touchstart" event means that you can count on the device being used to generate such events. It's like assuming because I know a few words of Spanish that I would prefer that you bring me the menu in Spanish rather than the one in English that I can read much more easily.

Some WebKit ports want have capabilities dynamically determined at runtime, with all traces of those capabilities hidden from the web platform. This is challenging to do in an efficient way. The normal way WebKit exposes DOM APIs doesn’t make it easy to have ones that are super-efficient but also dynamically present or not based on runtime capabilities. Because the Chrome team was vitally interested in having experimental features that could be turned on and off in their web browser, they made adding this feature to WebKit a high priority, but implemented it only for V8. That might have been because the engineers doing this work didn’t know how to do it efficiently for WebKit’s native JavaScript engine, or it might just be that they didn’t care about the feature outside their own use of it. I’m not sure everyone that is part of the WebKit project today agrees we want this feature in the WebKit tree. We should discuss what it would take to do this. The technical details are mostly about how to maintain the performance of the DOM bindings and also about how to keep things as simple as possible while adding even more switches. I don’t know if anyone at Apple is interested in working on this--it doesn’t seem that we need it for Mac or iOS at this time--and I also don’t know if anyone in the WebKit community has looked into what it will take to add this. There certainly will be a cost in complexity, but the benefit might be worthwhile.

PS: To make these websites work, we don’t want "ontouchstart" to return false, we want the expression '"ontouchstart" in window' to return false. It's no better and not helpful to talk about "typeof window.ontouchstart != 'undefined'" for multiple reasons. For one thing, presumably the websites doing these checks already exist, so it’s not like we can change them, so I think that stops the conversation before it even begins.
Comment 9 Carlos Garcia Campos 2015-10-02 00:49:30 PDT
*** Bug 146782 has been marked as a duplicate of this bug. ***
Comment 10 Carlos Garcia Campos 2015-10-02 01:12:42 PDT
It's sad but I'm afraid the touch events support is breaking more sites than what it fixes. I've found today some jquery elements that use "ontouchstart" in document.documentElement to decide whether to add listeners for mouse events or not. I agree that's wrong, because it's also assuming that if you have a touch input device you can't use any other non touch devices (like having a laptop with a touch screen and a usb mouse, which is my case). But unfortunately this wrong way is more and more common, so the only thing I can think of is disabling touch events support by default. People building for an embedded device with touch capabilities could enable it. My main concern is that we won't test it anymore. Another option could be making it a runtime feature that WTR can enable, like my patch tried to do but without enabling/disabling it automatically.
Comment 11 Michael Catanzaro 2015-10-02 07:01:42 PDT
In GNOME, our primary device targets are touchscreen laptops, non-touchscreen laptops, and desktops. We don't target embedded devices, but touch support is extremely important and must be present by default.

(In reply to comment #10)
> It's sad but I'm afraid the touch events support is breaking more sites than
> what it fixes. I've found today some jquery elements that use "ontouchstart"
> in document.documentElement to decide whether to add listeners for mouse
> events or not. I agree that's wrong, because it's also assuming that if you
> have a touch input device you can't use any other non touch devices (like
> having a laptop with a touch screen and a usb mouse, which is my case).

This is such an absurdly-common case that I think we should not attempt to support those sites. They can't possibly work properly in other browsers, regardless of whether ontouchstart is dynamically available or not, for the reason you mention. We should try to get in touch with the jquery developers to fix this before more sites start using it....
Comment 12 Jeff Fortin 2015-11-16 08:37:59 PST
This indeed affects apps like Kanboard that depend on jQuery to detect touch support, as seen on https://github.com/fguillot/kanboard/issues/1382#issuecomment-156870832
Comment 13 Michael Catanzaro 2016-01-02 10:32:06 PST
Comment on attachment 243496 [details]
Patch

Darin has several reservations about supporting runtime-enabled events, and it seems this patch is unlikely to fix websites in practice, correct? I think this requires at least discussion on webkit-dev, and possibly with W3C and other browsers.

If we are going to enable touch events at runtime, we should at least try to match the behavior of other engines. I am not sure, but I strongly suspect other engines are not checking whether the device uses a touchscreen, else the broken jQuery that makes a binary choice between using touch events or mouse events would not exist. Perhaps for compatibility, we should not be checking for whether a touchscreen is present, but for whether a non-touch input devices exist. E.g. if a mouse is plugged into your touchscreen computer, it's more important for the mouse to work than for the touchscreen to work.
Comment 14 David Gasperoni 2017-08-25 08:55:36 PDT
I was wondering if `'ontouchstart' in document.documentElement` should return true when in Responsive Design Mode. It would make sure many libraries (including Boostrap 3) are going to behave like on mobile Safari.

I was trying to debug a situation where a `.dropdown-backdrop` had higher z-index than `.dropdown-menu` (my fault) and I couldn't reproduce it in Safari (neither 10.1.2 or TP 38) while I could in Firefox 56.0b5. In Firefox's responsive mode, `'ontouchstart' in document.documentElement` return true when the page is loaded with a touch-enabled profile.

Maybe that would be an interesting proposition.
Comment 15 Michael Catanzaro 2017-08-25 09:38:37 PDT
(In reply to Michael Catanzaro from comment #13)
> Comment on attachment 243496 [details]
> Patch
> 
> Darin has several reservations about supporting runtime-enabled events, and
> it seems this patch is unlikely to fix websites in practice, correct? I
> think this requires at least discussion on webkit-dev, and possibly with W3C
> and other browsers.

My understanding is that this situation has changed in the past year, and Apple now wants to support runtime-enabled events. (Darin, is this true?) So I think it's fine for us to do this now.

> If we are going to enable touch events at runtime, we should at least try to
> match the behavior of other engines. I am not sure, but I strongly suspect
> other engines are not checking whether the device uses a touchscreen, else
> the broken jQuery that makes a binary choice between using touch events or
> mouse events would not exist. Perhaps for compatibility, we should not be
> checking for whether a touchscreen is present, but for whether a non-touch
> input devices exist. E.g. if a mouse is plugged into your touchscreen
> computer, it's more important for the mouse to work than for the touchscreen
> to work.

This is still seems very important to change.

(In reply to David Gasperoni from comment #14)
> I was wondering if `'ontouchstart' in document.documentElement` should
> return true when in Responsive Design Mode. It would make sure many
> libraries (including Boostrap 3) are going to behave like on mobile Safari.
> 
> I was trying to debug a situation where a `.dropdown-backdrop` had higher
> z-index than `.dropdown-menu` (my fault) and I couldn't reproduce it in
> Safari (neither 10.1.2 or TP 38) while I could in Firefox 56.0b5. In
> Firefox's responsive mode, `'ontouchstart' in document.documentElement`
> return true when the page is loaded with a touch-enabled profile.
> 
> Maybe that would be an interesting proposition.

I honestly have no idea what this means. Responsive Design Mode is a Safari developer option, right? Safari does not have to simultaneously support both touchscreen devices and traditional input devices due to Apple's decision not to manufacture touchscreen laptops, so I don't think Safari's behavior is at all relevant here. Currently ontouchstart always returns true if touch events are enabled at build time (so true on iOS, false on macOS, true for GTK+). The patch here would change it to return true only if touch events are supported by the user's monitor at runtime. My proposal is to change it to return true only if the user does not have a mouse or other pointer input device attached at runtime.
Comment 16 Michael Catanzaro 2017-08-25 09:42:40 PDT
Ah, I guess Responsive Design Mode is for developers testing their websites on phone-sized screens, right? If it's just a testing tool, then that might make sense indeed. Not sure.
Comment 17 Michael Catanzaro 2017-08-25 09:43:43 PDT
(In reply to Michael Catanzaro from comment #15)
> Currently ontouchstart always returns true if touch
> events are enabled at build time

Of course I mean that testing if it is present will return true. :)
Comment 18 David Gasperoni 2017-08-25 09:44:32 PDT
(In reply to Michael Catanzaro from comment #16)
> Ah, I guess Responsive Design Mode is for developers testing their websites
> on phone-sized screens, right? If it's just a testing tool, then that might
> make sense indeed. Not sure.

Yes, it's the equivalent of Firefox's [Responsive Design Mode](https://developer.mozilla.org/en-US/docs/Tools/Responsive_Design_Mode) and Chrome's [Device Mode](https://developers.google.com/web/tools/chrome-devtools/device-mode/).
Comment 19 Carlos Garcia Campos 2019-05-22 04:12:09 PDT
*** Bug 197773 has been marked as a duplicate of this bug. ***
Comment 20 Carlos Garcia Campos 2019-05-22 05:03:51 PDT
*** Bug 197735 has been marked as a duplicate of this bug. ***
Comment 21 Carlos Garcia Campos 2019-05-22 06:45:41 PDT
There are more and more sites broken because of this. I think the main concern is still the way to detect the touch mode to enable tocuh events at runtime. Checking if there's a touch device present as my path did is would break for user having a touch screen but using mainly a mouse (that used to be my case indeed). So, I think we have several options here:

 a) Enable/disable at runtime based on presence of a touch device. It will be still broken only for people having a touch screen by using a mouse.

 b) Stop building with touch events enabled by default. People building for specific hw with touch screen only can simply enable them at build time. Sites using touch events will be broken for people having touch screens. Most of the sties have a fallback mose using mose events, I think.

 c) Add a setting and expose the touch events at runtime depending on the setting (there's now EnabledBySetting). The setting could be auto by default, meaning it will be detected. It will be broken again for the case of using a mouse in a system with a touch screen, but at least users can force touch events to be enabled/disabled with the setting.

Any other idea?
Comment 22 Ryan DeBeasi 2019-05-22 07:13:37 PDT
My understanding is if a device has both a touch screen and a mouse, reporting support for touch events is valid behavior. So, I think option A would be a totally reasonable course of action.

For more granular touch/mouse checks, web developers can check for the `pointer` CSS media feature. CSS media queries for the pointer also return some inaccurate results on WebKit GTK+ - see Bug 197773.

https://developer.mozilla.org/en-US/docs/Web/CSS/@media/pointer
https://hacks.mozilla.org/2013/04/detecting-touch-its-the-why-not-the-how/
https://peterscene.com/detecting-touch-devices-2018-update/

Thank you so much for your time!
Comment 23 Carlos Garcia Campos 2019-05-23 06:12:08 PDT
Created attachment 370502 [details]
Patch

Let's try again
Comment 24 Carlos Garcia Campos 2019-05-23 06:12:51 PDT
*** Bug 141466 has been marked as a duplicate of this bug. ***
Comment 25 Michael Catanzaro 2019-05-23 08:17:51 PDT
Comment on attachment 370502 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370502&action=review

> Source/WebCore/page/RuntimeEnabledFeatures.h:536
> +    Optional<bool> m_touchEventsEnabled;

This seems confusing. The semantics of Optional don't lead themselves to a natural interpretation of this member variable. What does it mean for m_touchEventsEnabled to be unset: neither enabled nor disabled?
Comment 26 Carlos Garcia Campos 2019-05-23 08:19:16 PDT
(In reply to Michael Catanzaro from comment #25)
> Comment on attachment 370502 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370502&action=review
> 
> > Source/WebCore/page/RuntimeEnabledFeatures.h:536
> > +    Optional<bool> m_touchEventsEnabled;
> 
> This seems confusing. The semantics of Optional don't lead themselves to a
> natural interpretation of this member variable. What does it mean for
> m_touchEventsEnabled to be unset: neither enabled nor disabled?

When not explicitly set PlatformScreen is queried for touch capabilities.
Comment 27 Carlos Garcia Campos 2019-05-27 01:34:42 PDT
Committed r245788: <https://trac.webkit.org/changeset/245788>
Comment 28 Radar WebKit Bug Importer 2019-05-27 01:35:39 PDT
<rdar://problem/51153897>
Comment 29 Simon Fraser (smfr) 2019-05-27 08:43:36 PDT
This broke internal Apple builds. Ideally it would have had review by an Apple reviewer, since the Touch Events code can only be tested in internal Apple builds. I'll try to fix it.
Comment 30 Simon Fraser (smfr) 2019-05-27 09:01:28 PDT
https://trac.webkit.org/changeset/245793/webkit
Comment 31 Carlos Garcia Campos 2019-05-27 11:24:47 PDT
(In reply to Simon Fraser (smfr) from comment #29)
> This broke internal Apple builds. Ideally it would have had review by an
> Apple reviewer, since the Touch Events code can only be tested in internal
> Apple builds. I'll try to fix it.

Oh, I'm sorry, I waited for 3 days before landing it, precisely to give time to Apple reviewers to comment.
Comment 32 Carlos Garcia Campos 2019-05-27 11:25:04 PDT
(In reply to Simon Fraser (smfr) from comment #30)
> https://trac.webkit.org/changeset/245793/webkit

Thanks!