"Enable touch events" flag / detection should only influence TouchEvent API feature detection
Reported by
chefjoep...@gmail.com,
Sep 6 2016
|
|||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36 Steps to reproduce the problem: 1. Launch Chrome browser on any Windows machine (with default Chrome settings). 2. Plug in a SMART Board (or a HID device) to the machine. 3. Try to use touch to search or change the URL or touch anything on the browser page. What is the expected behavior? All touch events should work as expected. I should be able to focus on the address bar and enter a new URL, change tabs, or touch any of the widgets or objects on the browser page. What went wrong? Touch events are not getting through to any of the widgets except the title bar and the buttons in the top right to minimize, restore and close the window. I should be able to change the URL in the address bar, change to different tabs and interact with the widgets/objects on the browser page. If you re-launch the browser while the HID device (or SMART Board) is plugged in then everything works as expected. Did this work before? N/A Chrome version: 53.0.2785.89 Channel: stable OS Version: Any Flash Version: Shockwave Flash 22.0 r0 This is a problem when the flag for "Enable touch events" is set to automatic. If set to enabled then the problem goes away. At the very least, we would like to be able to deploy this flag setting to multiple machines as a per machine setting for users in companies. Not sure if this has always been this flaky but we, at SMART, seem to be getting more user complaints pertaining to this issue.
,
Sep 7 2016
SMART has confirmed this with other commonly used touch kits from other vendors, it appears to be a general problem with how touch panels are handled with Chrome.
,
Sep 8 2016
It sounds like this is at least in part issue 392584 . But that only explains why web pages wouldn't respond to touch after plugging in a touch screen without restarting Chrome. I think ensuring that the native UI elements work with touch when a screen is attached dynamically is valuable, even if the TouchEvent API will not be enabled (eg. sites depending only on pointer events or scrolling+compat mouse events shouldn't be affected by the underlying web compat issue discussed in issue 392584 ). tdresser/jschuh: any idea who could look into the native UI component of this issue?
,
Sep 8 2016
IIRC there's also some history here with this flag being used for BOTH disabling the TouchEvent API AND disabling touch support elsewhere in the UI, but I thought that was separated by sadrul@ at one point years ago. Given the imminent launch of pointer events, I believe the ideal behavior is for touch support generally to always be enabled, and only the TouchEvents API to be controlled by this switch. It's not immediately clear to me from the code if this is the case at the moment or not. At a minimum some clarifications/cleanups are necessary, eg. WebPreferences::touch_enabled should be renamed "touch_events_enabled", the description in about flags should say explicitly that this is about the TouchEvents API (not "touch support"), and some comments in touch_enabled.cc should be clarified.
,
Sep 8 2016
Yes, the switch should minimally suppress TouchEvent firing, w/o affecting the UI or PointerEvents. The same would true for touch gestures (always on), right? If a user can drag tabs with touch, he would naturally expect the page to be scrollable with touch IMO.
,
Sep 8 2016
Confirmed this is a long standing issue and not a regression, sorry about the confusion.
,
Sep 8 2016
Is there anything needed to be performed by SMART to help? Sounds like we have identified that this has been happening since at least Chrome version 37/38.
,
Sep 8 2016
,
Sep 8 2016
The input-dev team will dig into cleaning up this flag, and identifying where it's being used when it shouldn't be (in issue 645121 ). This may involve filing some bugs that would be better investigated by the Windows team, but we'll see when we get there.
,
Sep 28 2016
Has there been any update on this issue? We have been getting a lot more questions from more people/customers at SMART about this issue and would like to have some update on the progress for this bug.
,
Sep 28 2016
Hi chefjoepharaoh@: We will work on the root cause of the problem through crbug.com/632881 next month. We will leave this bug open until then---to verify that our fix covers the problem you reported here.
,
Oct 13 2016
Clarifying the title based on the discussion above. Regardless of whether we detect a touch screen at startup, touch should work for the UI and dispatch Pointer Events. This should fix much of the problem described here. This is also important for proper Pointer Events support so tentatively marking Pri-1 for M56. Mustaq/Dave/Tim feel free to downgrade to Pri2 if you disagree. There's still, however, a fundamental problem with the TouchEvent API (i.e. "'ontouchstart' in window") which is separate. Fixing this bug will not help websites that depend on TouchEvents - they will continue not to function with touch without either 1) restarting the browser after a touch screen is attached, or 2) enabling chrome://flags#touch-events. That is tracked by issue 632881 but we have no immediate path for fixing it due to website compatibility problems. Hopefully more websites will use the Pointer Event API when present instead of relying on Touch Events.
,
Oct 14 2016
I agree this is a Pri-1 for M56. sunyunjia@: Let's sync next week to hash out the details.
,
Oct 18 2016
I did some investigation to try and better pinpoint when\how this bug was introduced. Unfortunately it is conflated with other Windows HID issues and difficult to isolate on its own, we have conflicting data. The steps are a 100% repro case that isolates the issue, though.
,
Oct 25 2016
,
Nov 3 2016
Hi Team, Do we have any updates here for the SMART Tech team?
,
Nov 3 2016
Hi pvansh: We are working on it, should be fixed soon.
,
Nov 3 2016
Sandra is working on a patch here: https://codereview.chromium.org/2467913002/
,
Nov 8 2016
Thanks for the update, folks. Let me know if you want us to try out a beta version or something.
,
Nov 21 2016
,
Nov 23 2016
Can someone verify the patch once the CL is committed? M56 Beta promotion is scheduled during the first week of December.It would be great to have the fix verified and merged to M56 branch ASAP.
,
Nov 23 2016
Our fix is ready crrev.com/2467913002, blocked by a single benchmark failure which seems independent. We are working closely with the benchmark owner, hopefully we will be quick after this Thanksgiving week.
,
Nov 23 2016
,
Nov 24 2016
We can test once it is on the dev channel.
,
Nov 24 2016
brentadam@: The fix hasn't landed yet, we need one last approval.
,
Nov 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a880dd75bf766d22cbe4ba79bb168f7ea38c524 commit 7a880dd75bf766d22cbe4ba79bb168f7ea38c524 Author: sunyunjia <sunyunjia@chromium.org> Date: Thu Nov 24 22:05:17 2016 Touch event flag should control only DOM event firing. Currently there is no way to disable TouchEvent API support without affecting either PointerEvents (for touches) or touch support in browser UI. With PointerEvents shipping in M55, it now makes the most sense to turn on touch support in browser unconditionally and let the --touch-events flag control only the firing of DOM events. This CL also renames the flag to clarify its purpose. BUG= 644318 Review-Url: https://codereview.chromium.org/2467913002 Cr-Commit-Position: refs/heads/master@{#434399} [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/chrome/app/generated_resources.grd [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/chrome/browser/about_flags.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/browser/renderer_host/legacy_render_widget_host_win.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/browser/renderer_host/render_view_host_impl.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/public/common/common_param_traits_macros.h [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/public/common/web_preferences.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/public/common/web_preferences.h [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/renderer/render_view_impl.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/content/test/test_blink_web_unit_test_support.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/core/dom/Document.idl [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/core/dom/GlobalEventHandlers.idl [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/core/events/EventAliases.in [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/core/input/TouchEventManager.cpp [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/web/DevToolsEmulator.cpp [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/web/DevToolsEmulator.h [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/Source/web/WebRuntimeFeatures.cpp [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/third_party/WebKit/public/web/WebRuntimeFeatures.h [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/tools/metrics/histograms/histograms.xml [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/tools/perf/page_sets/system_health/browsing_stories.py [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/ui/base/BUILD.gn [delete] https://crrev.com/d0124d407177d3a9e0763682a791ab17b78394c0/ui/base/touch/touch_enabled.cc [delete] https://crrev.com/d0124d407177d3a9e0763682a791ab17b78394c0/ui/base/touch/touch_enabled.h [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/ui/events/devices/x11/touch_factory_x11.cc [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/ui/events/devices/x11/touch_factory_x11.h [modify] https://crrev.com/7a880dd75bf766d22cbe4ba79bb168f7ea38c524/ui/views/win/hwnd_message_handler.cc
,
Nov 25 2016
As Rick pointed out in the CL above, we will need to restore the auto behavior in the flag.
,
Nov 28 2016
M56 beta launch is next week.Your bug is labelled as Release Block beta, please make sure to land the fix by first week of December.
,
Dec 1 2016
Talking about this more we realized we don't even really want the "touch event API" support to be dynamic (you can always open devtools at use mobile emulation to generate touch events). It's just the feature detection (eg. 'ontouchstart' in window) that needs to be dynamic due to 632881. If web developers are following our advice and being input agnostic (always listening for touch and mouse input - https://hacks.mozilla.org/2013/04/detecting-touch-its-the-why-not-the-how/), and are using modern addEventListener DOM eventing (instead of DOM0 ontouchstart= style), then the feature detection should be largely irrelevant to them.
,
Dec 1 2016
Thanks for the investigation. M56 Beta promotion is scheduled on Dec 06 , please make sure to get this resolved ASAP.
,
Dec 1 2016
Controlling only the feature detection makes sense here, thanks for the suggestion. Hopefully our feature-detection getting orthogonal to TouchEvent API support won't be bad in the long term, like won't trigger ugly hacks to detect API support. Let's skip M56 in case we need more work here.
,
Dec 1 2016
,
Dec 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78171bf829d1ae47b7d7237c08878783a2cdadb5 commit 78171bf829d1ae47b7d7237c08878783a2cdadb5 Author: sunyunjia <sunyunjia@chromium.org> Date: Fri Dec 02 02:04:55 2016 Reset the default value of touch event flag back to "Auto". This is a partial revert of issue 2467913002. https://codereview.chromium.org/2467913002 In the previous patch, we set the default value of touch-event-api as enabled. However, this may cause troubles due to http://crbug.com/392584 . Basically there are still a bunch of websites on which mouse support is broken when the TouchEvents API is present. So we need to retain the "auto" behavior as the default. We also keep firing DOM events as before even when the flag says disabled. BUG= 644318 , 669386 , 669132 Review-Url: https://codereview.chromium.org/2531413002 Cr-Commit-Position: refs/heads/master@{#435821} [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/chrome/app/generated_resources.grd [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/chrome/browser/about_flags.cc [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/content/browser/renderer_host/render_view_host_impl.cc [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/third_party/WebKit/Source/core/input/TouchEventManager.cpp [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in [modify] https://crrev.com/78171bf829d1ae47b7d7237c08878783a2cdadb5/tools/metrics/histograms/histograms.xml
,
Dec 2 2016
We are now back to original flag state. Only remaining task in this bug now is renaming (again :( ) the internal states to represent the new meaning, i.e. enable "touch feature detection".
,
Dec 6 2016
,
Dec 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa6a8afcca5bb185d7c0a89db6775073b530989e commit fa6a8afcca5bb185d7c0a89db6775073b530989e Author: sunyunjia <sunyunjia@chromium.org> Date: Fri Dec 09 04:25:47 2016 Rename TouchEventAPI to TouchEventFeatureDetection We don't really want the "touch event API" support to be dynamic (you can always open devtools at use mobile emulation to generate touch events). It's just the feature detection (eg.'ontouchstart' in window) that needs to be dynamic due to 632881. BUG= 644318 Review-Url: https://codereview.chromium.org/2547373002 Cr-Commit-Position: refs/heads/master@{#437464} [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/chrome/browser/about_flags.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/chrome/browser/apps/guest_view/web_view_browsertest.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/chrome/browser/chromeos/login/chrome_restart_request.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics_unittest.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/browser/renderer_host/input/touch_action_browsertest.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/browser/renderer_host/input/touch_input_browsertest.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/browser/renderer_host/render_view_host_impl.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/browser/web_contents/web_contents_view_aura_browsertest.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/public/common/common_param_traits_macros.h [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/public/common/content_switches.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/public/common/content_switches.h [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/public/common/web_preferences.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/public/common/web_preferences.h [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/renderer/render_view_impl.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/shell/app/shell_main_delegate.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/content/test/test_blink_web_unit_test_support.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/core/dom/Document.idl [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/core/dom/GlobalEventHandlers.idl [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/core/events/EventAliases.in [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/web/DevToolsEmulator.cpp [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/web/DevToolsEmulator.h [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/Source/web/WebRuntimeFeatures.cpp [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/third_party/WebKit/public/web/WebRuntimeFeatures.h [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/tools/metrics/histograms/histograms.xml [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/ui/events/event_switches.cc [modify] https://crrev.com/fa6a8afcca5bb185d7c0a89db6775073b530989e/ui/events/event_switches.h
,
Dec 9 2016
,
Dec 13 2016
chefjoe... are you able to confirm that this addresses your smart board issue? It has been in chrome canary for a few days and we'd like to ensure that we addressed the issue you were experiencing.
,
Dec 14 2016
I did some testing on this on windows 7 and 4 different boards in HID mode. I first reproduced the issue with Version 54.xxx and then installed chrome canary and verified the issue is fixed. I tried to reproduce it on Windows 10 with Version 54 but it worked properly. Tried chrome canary on Windows 10 and it worked. Touch events worked as expected. I only had time to do about an hour of testing but it looks fixed.
,
Dec 14 2016
Sounds like John tested the bug and is fixed in Chrome Canary. When do we expect this fix to be in a released version, or which version is this fix expected to be released?
,
Dec 14 2016
Thanks for verifying the fix. The change will be available in Chrome 57 stable in mid-March, and in 57 beta in early February. https://www.chromium.org/developers/calendar
,
Dec 22 2016
,
Jan 18 2017
|
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by chefjoep...@gmail.com
, Sep 6 2016