Its a regression issue. 

Please find the log & video @ http://go/chrome-androidlogs1/6/642698

able to repro on Samsung Galaxy J5 / 53.0.2785.90

acindhe@ next time ask ET team to add proper bisect info, if it is regression. Also bisect range seems to be wrong.

I have checked on Asus Zenfone (ZE551ML)/ LRX21V

M37 (system image) - works fine
52.0.2743.98 (stable) - fine
53.0.2744.0 - Can't get those builds -install fails due to API level set during that time
53.0.2784.1 - broken (last build before branch cut)
53.0.2785.10 - broken(first build after branch cut)
53.0.2785.57 - broken
53.0.2785.80 - broken(last beta)

It is broken from M53. Working fine on M52.

Bisect range: https://chromium.googlesource.com/chromium/src/+log/52.0.2743.98..53.0.2784.1?pretty=fuller&n=10000  (it has lot of drop-down changes not from webview directly)

doesn't look like a blink issue

that bisect range in #4 is pretty useless though, what does api level has to do with anything on an L device?

broken between 53.0.2774.0 and 53.0.2779.0, no archived builds in between :/

A little bit more narrow range: https://chromium.googlesource.com/chromium/src/+log/53.0.2774.4..53.0.2779.0?pretty=fuller&n=10000

Bisected to https://codereview.chromium.org/2070053004

that's interesting...

hmm... in this should be a regular touch event from user, not one generated by js

page is using this js library called FastClick to re-dispatch events, and the re-dispatched event is no longer considered trusted: https://www.chromestatus.com/features/5718803933560832

I'm trying to work out what the re-dispatched event is exactly, because clearly it's not a click..

should we remove the rbs, since now we know this is intended behavior and that library needs to be updated to fix it?

the generated event is mousedown, which apparently applies only to <select> tags on android:

FastClick.prototype.determineEventType = function(a) {
    "use strict";
    return this.deviceIsAndroid && "select" === a.tagName.toLowerCase() ? "mousedown" : "click"
}

https://github.com/ftlabs/fastclick/blob/master/lib/fastclick.js#L311

so this would *only* have broken this library on android, do we trust the chrome beta population is large enough to catch regressions like that..?

-RVG, nothing private here

Removing rbs label due to wai.

Putting rbs label back pending further evaluation on potential impact of change.

There's definitely a risk here due to the Android UA check and little usage of beta on Android.  Some initial thoughts off the top of my head (since I understand we're looking for a decision ASAP):

- It's been years since fastclick was required on Chrome.  See https://developers.google.com/web/updates/2013/12/300ms-tap-delay-gone-away?hl=en.  If you follow the advice there (either viewport or touch-action), fastclick detects that and gets out of the way (notNeeded returns true).  This has been true of fastclick for at least the last couple years IIRC.
- There are a bunch of other downsides of depending on fastclick (eg. don't get the benefit of touch adjustment, tap suppression during fling, etc.).  We really do want to move the ecosystem off relying on it.  The alternatives are simple and now supported on all browsers (non-zoomable viewport or touch-action:manipulation).
- We found that allowing script to programatically open select boxes was actually a security risk (with some at least partially compelling proof-of-concept exploits in the wild).  So there'd be a security risk to backtracking now.

So without more concrete data on the compat risk my judgement is that we should go ahead with the release and deal with the fall-out.

Note that we have UMA data that will provide an upper bound on the risk here (which was used to justify a WontFix of  issue 520520 ), but the UMA dashboard appears down at the moment so I can't get the Android-specific numbers.  When it comes back up someone can look at the WebCore.FeatureObserver entry for UntrustedEventDefaultHandled.

For Chrome-wide, it's around 0.015% of page loads.  I verified that sending a synthetic mousedown to a <select> element does get counted in this bucket.  There are other things that trigger it, but anecdotally I expect the <select> case is at least half the usage.  We just need to verify that the Android-specific number isn't dramatically higher.

Meant to include this link for the chrome-wide UseCounter data: https://www.chromestatus.com/metrics/feature/timeline/popularity/1372

Note that UMA doesn't include WebView.  If the WebView impact in particular was high we could:

- Disable the TrustedEventsDefaultAction RuntimeEnabledFeature for WebView temporarily (for 53)
- In a future Chrome release add a targetSdk compat hack to preserve the untrusted mousedown behavior on <select> for apps built against Android versions N or earlier.

+aelias who probably has an opinion on this.

UMA data (UntrustedEventDefaultHandled UseCounter) shows up to ~0.04% of page views on Android could be impacted.  Most of those cases are probably not noticable (things other than <select> that don't matter to the UX), but we can't tell how much.  Still that's frighteningly high.

amineer@ and I agreed that we'll proceed with a small Chrome Android stable push.  But we have until Monday to merge a fix to M53 for the next (larger) push if we want to disable this to buy us some time...

Simple repro: http://output.jsbin.com/baruho

Works fine in Mobile Edge for some reason (from the fastclick code I would have thought it would hit the same issue).

Filed https://github.com/ftlabs/fastclick/issues/497 to make FastClick authors aware of the issue (I've talked to them a bunch in the past - they said they prefer to get out of the way when the browser has a better solution).

The Clank beta population is not *that* small and there's no reason to think the prevalence is higher on WebView.  Just that it so happens we got one bug filed for WebView.

I guess my fear level is not as high as yours in this case.  I think we could push this as planned in 53, it would cause a certain amount of breakage but it wouldn't the kind of flood of complaining that would make us feel forced to respin.  The end result of the breakage would be developers stop doing the thing we think they need to stop doing for security reasons.  And maybe develop a general distrust of fastclick.js and stop using it so much.  All's well that ends well.

Maybe we could go with targetSdkVersion approach just because that's a nicer way of rolling out breaking changes in general, WebView developers find it harder to roll out updates for emergency fixes, and we already have a setting we can hook up.  Although it would mean maintaining and unit testing both paths indefinitely, and it's your call on whether that would be painful or not.

Bo indicated that the site in this webview was actually fetched via a network connection and the files werent packaged in the apk. So fixing this cause should just be patching the website correctly. 

aelias@ thanks - maybe I'm being too cautious.  Certainly the fact that we haven't heard complaints from clank beta suggests this can't be THAT common (certainly if we were really breaking UX on even 0.01% of page views I'd expect to have gotten user complaints already).

dtapuska@ my intuition (from the anecdotes we've seen) was that a non-trivial fraction of all UntrustedEventDefaultHandled cases would be the <select> one, which is why I'm so concerned about the 0.04% number.  Are you aware of other cases which may dominate that metric?  Maybe even trivial cases (or some popular site) are hitting it somehow in a way that's not user-visible?  I tried browsing a few sites I thought might hit this, but didn't get any hits.

If we have a reason to believe that the vast majority of that 0.04% number will not impact the user experience, then I'm OK with just waiting to see if we get many complaints.

Dave and I have talked a bunch about this.  Summary of conclusions:

1. Dave will add (and merge to 54) a UseCounter to monitor how often a <select> receives an untrusted mousedown.  That (in a couple weeks) should give us a tight bound on the specific breakage here (how much of the 0.04% is really this case).

2. Given we haven't heard complaints for clank, we think the risk there is low.  Most sites using fastclick will be using a mobile viewport and so avoid this issue.  Absent further complaints, we see no reason to try to rush a change in to clank for 53.

3. We're more concerned about WebView.  Perhaps there's a reason why WebView apps are less likely to have the mitigation above?  In particular, the app that this bug was opened for does not appear to have zoomed-out content in the webview - why isn't it hitting the viewport-related "notNeeded" check in fastclick telling it to get out of the way?  If WebView apps are much more likely to be broken, then we could merge a tiny low-risk patch into 53 which re-enables just the <select> case (potentially just for webview).  I tried to debug the app but couldn't get Chrome DevTools to show the WebView instance.  Dave is going to dig further.

4. I will write up (and test) guidance on https://github.com/ftlabs/fastclick/issues/497 for developers affected by this issue.  It _should_ be as simple as setting the touch-action style to cause FastClick to get out of the way (https://developers.google.com/web/updates/2013/12/300ms-tap-delay-gone-away?hl=en)

5. I'll see if I can get my FastClick pull-request (https://github.com/ftlabs/fastclick/pull/499) landed anyway, so that if people do try updating FastClick they won't ever have problems with it on Chrome again (disables it completely for modern Chrome, Safari and Edge).

Not sure if it's the case for the app in question, but WebView apps often have content baked into the app itself that may not have been updated for a very long time (years in some cases), and so may be using ancient, dumb versions of libraries that don't detect when this kind of special treatment is unnecessary correctly/at all.

> 3. We're more concerned about WebView.  Perhaps there's a reason why WebView apps are less likely to have the mitigation above?  In particular, the app that this bug was opened for does not appear to have zoomed-out content in the webview - why isn't it hitting the viewport-related "notNeeded" check in fastclick telling it to get out of the way?

It could be due to https://developer.android.com/reference/android/webkit/WebSettings.html#setUseWideViewPort(boolean) .  If this setting is true, WebView's viewport behaves like Clank's (default 980 layout width -- that's why it's named wide viewport -- and also respects viewport tag).  If the setting is false, WebView behaves like a desktop browser: layout width is the DIP-adjusted WebView width, and the viewport tag is not parsed.

The default is false (and this default is a bit alarming for this purpose -- it means the path of least resistance to write WebView content is to *not* have a viewport tag).  If you observe that the Abof app HTML doesn't have a meta viewport tag, this would be why.

> I tried to debug the app but couldn't get Chrome DevTools to show the WebView instance.  Dave is going to dig further.

You'll need a userdebug device. Or maybe easier, the app loads this page (among others) into a webview: https://live-cdn.me-tail.net/wanda/ui/19d7fdfd-29eb-43fc-929e-45b321074f2b/en-US

So that specific URI in #31 doesn't have a viewport tag at all. Whereas if you use their web portal which shows very similar content they do have the viewport tag.

Where as content like:
http://www.abof.com/product/IINKA16AWFSTP9P13809-Indian-Ink-Women-Black-and-Orange-Printed-Reversible-A-line-Top

Does provide a viewport tag. I'll double check on a userdebug build.

Confirmed
* use_wide_viewport remains false for this app
* page does not have a meta viewport tag

> * page does not have a meta viewport tag

I tested that in the app, not just opening that link from chrome

https://live-cdn.me-tail.net/wanda/ui/19d7fdfd-29eb-43fc-929e-45b321074f2b/en-US indeed doesn't have a viewport tag.  Yeah, OK, so I'm convinced my useWideViewport story is what's going on.  The Clank Beta experience then is not representative of the prevalence in WebView.  And the WebView beta population is miniscule, so if our internal tester happened to discover a problem, it may be just the tip of the iceberg.

Thanks for the details.  Agreed this difference between WebView and clank is cause for concern.

When setUseWideViewPort is false, is double-tap-to-zoom (and so the click delay) still enabled?  Ideally there'd be no need for something like FastClick at all in the default WebView case.  The place we determine this for clank is  VisualViewport::shouldDisableDesktopWorkarounds - perhaps that needs to be updated for WebView?  If so we should file a separate bug (I don't think it'll help here).

The only other mitigation that could help is if  "document.documentElement.scrollWidth <= window.outerWidth" was usual true in these cases.  Then we'd hid the early-out condition here (at least in fastclick from the last couple years): https://github.com/ftlabs/fastclick/blob/master/lib/fastclick.js#L759

Assuming neither of the above lead to us believing this app is rare, then Dave and I agree that we should proceed down the path of mitigating the risk just for WebView.  In particular:
 1. land a blink patch tomorrow which re-enables this specific case (untrusted mousedown opens select) only for webview (is there an #ifdef we can use temporarily?).
 2. Merge the above back to Chrome 53, block the full WebView release on it
 3. Replace the above hack with a proper targetSdk check in Chrome 54 (or 55)

> When setUseWideViewPort is false, is double-tap-to-zoom (and so the click delay) still enabled?

According to this code, no:
https://cs.chromium.org/chromium/src/android_webview/java/src/org/chromium/android_webview/AwSettings.java?rcl=0&l=1762

>> When setUseWideViewPort is false, is double-tap-to-zoom (and so the click delay) still enabled?
> According to this code, no

Ok, good.  That means developers shouldn't be intentionally using FastClick in their pages designed specifically for webview (i.e. those unlikely to have mobile viewports).  If we're sure this is true then this should mitigate the risk significantly.

bo: do you mind double-checking that this is indeed true by loading http://output.jsbin.com/vimuxew in a simple webview?  You should see a few ms, not ~200ms (there's no viewport tag or touch-action so it's all up to setUseWideViewPort logic).  I'm waiting for my device to re-image to userdebug in order to be able to debug a webview app.

Note that I was wrong about which code controls the click delay - it's not the shouldDisableDesktopWorkarounds.  It's in RenderWidgetHostViewAndroid::OnFrameMetadataUpdated.  This and the WebView code both boil down to calls to GestureProvider::SetDoubleTapEnabled, so they should have the same impact on the click delay.

> bo: do you mind double-checking that this is indeed true by loading http://output.jsbin.com/vimuxew in a simple webview?

delay around 10ms after things settle down

> only for webview (is there an #ifdef we can use temporarily?)

No.  Monochrome is shipping now so WebView and Chrome are literally the same binary code.  I recommend using the runtime setting "wide_viewport_quirk".  This is always true on WebView and always false on every other configuration.  (Note: this is separate from "use_wide_viewport" which simply maps to the value of WebSettings.setUseWideViewPort() .  wide_viewport_quirk is needed because my description of the meaning of wide_viewport above was an oversimplification, and WebView has a few additional viewport tag interpretation quirks not found in Clank.)

> delay around 10ms after things settle down

Perfect, so that confirms that WebView doesn't have a click delay by default.  So it looks like this app was sharing some code with it's website (enough to get the FastClick by accident) but not all the html (not enough to get the viewport tag).  Bo/Alex any intuition on how common this is likely to be?  Concerning enough to warrant a last-second M53 patch as I suggested in #2?  Or probably rare enough that we can just wait and see if there's enough reported breakage to justify a targetSdk check in 54?

My intuition is weak, I have no idea how common.  I'm inclined to go with the safe revert option and targetSdk option (or perhaps something clever like report an artificial viewport tag to JS examining it?  rational or not?).  Let's say the issue only affects this app (5 million downloads) and say two other apps of the same magnitude.  It's still more than a little user and developer-hostile to break several of their flows (could be a really important flow like login or checkout) and have them scramble to figure out WTF happened and push emergency update and for what?

It may be different if this affected Chrome.  There we have no way to deploy breaking changes with a soft landing.  But with the smooth and easy ramp of targetSdkVersion available, I don't really see a reason to throw anybody of a cliff.

Sounds reasonable to me, thanks Alex.

On the "clever" front, Dave and I brainstormed this a bit earlier.  Some ideas:
 - use_wide_viewport could generate a corresponding viewport tag when none exists
 - we could try to fix the window.outerWidth value so that the "document.documentElement.scrollWidth <= window.outerWidth" condition is satisfied

But both of these risk having some unintended compat side-effects.  Just having the targetSdk hack seems strictly better.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b677a41a4e30b6bf68bc4cd2a9ec8876d5e2f7ea

commit b677a41a4e30b6bf68bc4cd2a9ec8876d5e2f7ea
Author: dtapuska <dtapuska@chromium.org>
Date: Fri Sep 02 05:23:15 2016

Add UMA metric to track usage of sending a mousedown to select elements.

There is potential around the fast click library trapping touchend events
and generating mousedown events for android chrome.

Track the usage of this behaviour we are seeing.

BUG= 642698 

Review-Url: https://codereview.chromium.org/2303013002
Cr-Commit-Position: refs/heads/master@{#416193}

[modify] https://crrev.com/b677a41a4e30b6bf68bc4cd2a9ec8876d5e2f7ea/third_party/WebKit/Source/core/events/EventDispatcher.cpp
[modify] https://crrev.com/b677a41a4e30b6bf68bc4cd2a9ec8876d5e2f7ea/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/b677a41a4e30b6bf68bc4cd2a9ec8876d5e2f7ea/tools/metrics/histograms/histograms.xml

I've posted

https://codereview.chromium.org/2300633007

Bo did a quick test and the app sees to be working.
aelias@ can you do a review?

Pre-emptively approving merge as I will be OOO this afternoon.  Please make sure it's merged by EOD today.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c254feb3c58b5a9223591084e77d093173d582b

commit 0c254feb3c58b5a9223591084e77d093173d582b
Author: dtapuska <dtapuska@chromium.org>
Date: Fri Sep 02 20:10:18 2016

Enable untrusted mousedown on select boxes for webview

We have found that some webviews are using fastclick inside them and
due to a risk of breakage we are going to allow mousedown on selects
even if they are untrusted.

BUG= 642698 

Review-Url: https://codereview.chromium.org/2300633007
Cr-Commit-Position: refs/heads/master@{#416330}

[modify] https://crrev.com/0c254feb3c58b5a9223591084e77d093173d582b/third_party/WebKit/Source/core/events/EventDispatcher.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12e9fa3d012bcaa1a3aecea1a35c68309e8b8bd9

commit 12e9fa3d012bcaa1a3aecea1a35c68309e8b8bd9
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Sep 02 20:16:04 2016

Enable untrusted mousedown on select boxes for webview

We have found that some webviews are using fastclick inside them and
due to a risk of breakage we are going to allow mousedown on selects
even if they are untrusted.

BUG= 642698 

Review-Url: https://codereview.chromium.org/2300633007
Cr-Commit-Position: refs/heads/master@{#416330}
(cherry picked from commit 0c254feb3c58b5a9223591084e77d093173d582b)

Review URL: https://codereview.chromium.org/2305923003 .

Cr-Commit-Position: refs/branch-heads/2785@{#816}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/12e9fa3d012bcaa1a3aecea1a35c68309e8b8bd9/third_party/WebKit/Source/core/events/EventDispatcher.cpp

Issue does not repro on latest M53 - 53.0.2785.97. Thanks 

We still see this issue on latest webview M54. Could we merge the fix to M54?

Your change meets the bar and is auto-approved for M54 (branch: 2840)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59252ee321674d21f91662d3e6188975e34c83f7

commit 59252ee321674d21f91662d3e6188975e34c83f7
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Thu Sep 08 19:49:29 2016

Enable untrusted mousedown on select boxes for webview

We have found that some webviews are using fastclick inside them and
due to a risk of breakage we are going to allow mousedown on selects
even if they are untrusted.

BUG= 642698 

Review-Url: https://codereview.chromium.org/2300633007
Cr-Commit-Position: refs/heads/master@{#416330}
(cherry picked from commit 0c254feb3c58b5a9223591084e77d093173d582b)

Review URL: https://codereview.chromium.org/2316373005 .

Cr-Commit-Position: refs/branch-heads/2840@{#244}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/59252ee321674d21f91662d3e6188975e34c83f7/third_party/WebKit/Source/core/events/EventDispatcher.cpp

Fix verified on webview latest M54 on MotoX(2ndGen)/6.0.0

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59252ee321674d21f91662d3e6188975e34c83f7

commit 59252ee321674d21f91662d3e6188975e34c83f7
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Thu Sep 08 19:49:29 2016

Enable untrusted mousedown on select boxes for webview

We have found that some webviews are using fastclick inside them and
due to a risk of breakage we are going to allow mousedown on selects
even if they are untrusted.

BUG= 642698 

Review-Url: https://codereview.chromium.org/2300633007
Cr-Commit-Position: refs/heads/master@{#416330}
(cherry picked from commit 0c254feb3c58b5a9223591084e77d093173d582b)

Review URL: https://codereview.chromium.org/2316373005 .

Cr-Commit-Position: refs/branch-heads/2840@{#244}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/59252ee321674d21f91662d3e6188975e34c83f7/third_party/WebKit/Source/core/events/EventDispatcher.cpp