New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 733736 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 661629



Sign in to add a comment

Simulated shift-clicks bypass tab-under blocking

Project Member Reported by mustaq@chromium.org, Jun 15 2017

Issue description

The spec requires that the default action of a "click" event is executed even for an untrusted event:
https://www.w3.org/TR/uievents/#trusted-events

Because of this, chrome currently shows a "double-navigation" behavior when a link's onclick handler sends a simulated ctrl-click or shift-click to another link.

Looks like the user gesture is not consumed correctly in this case.

http://output.jsbin.com/sejiwob
 

Comment 1 by mustaq@chromium.org, Jun 15 2017

Cc: bmcquade@chromium.org japhet@chromium.org
Labels: OS-All
Owner: mustaq@chromium.org
Interesting: modified the repro to try click two links, which seems to work fine.

This looks like a crack in gesture consumption: I suspect either a race condition somewhere, or a result of our two-level consumption.

Repros in current stable.
Labels: UserActivation

Comment 3 by mustaq@chromium.org, Jan 10 2018

While debugging UserActivation v2 failures, found a wrong test that claims to suppress popups from an untrusted "click" that is dispatched from the handler of a trusted "click":
  fast/events/popup-blocked-from-untrusted-mouse-click.html

This test passes for a different reason: user activation is currently suppressed beyond a setTimeout call depth of 1, see Issue 760848#c4.  As a result, it fails when the setTimeout call at "body onload=..." is dropped.

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 11 2018

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

commit 2a45020311f5d70112724ebb42bb56eae8c7e593
Author: Mustaq Ahmed <mustaq@google.com>
Date: Thu Jan 11 15:33:15 2018

Removed a broken test.

The test have been passing because of a depth-of-timer limit. We
have fast/events/popup-blocking-timers*.html for this already.

Bug:  733736 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Id87c1f95d7af22b10d2511b501dfcde3961becf6
Reviewed-on: https://chromium-review.googlesource.com/861082
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528632}
[modify] https://crrev.com/2a45020311f5d70112724ebb42bb56eae8c7e593/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[delete] https://crrev.com/1923a62fbb69f8d57594e7c3cd6f7dfcdbe9d1e5/third_party/WebKit/LayoutTests/fast/events/popup-blocked-from-untrusted-mouse-click-expected.txt
[delete] https://crrev.com/1923a62fbb69f8d57594e7c3cd6f7dfcdbe9d1e5/third_party/WebKit/LayoutTests/fast/events/popup-blocked-from-untrusted-mouse-click.html

Comment 5 by mustaq@chromium.org, Jan 16 2018

Summary: Chrome allows double navigation through simulated clicks (was: Simulated clicks show a popup like behavior)
Cc: mustaq@chromium.org
Owner: csharrison@chromium.org
I think this is expected behavior, but your example in Link 4 bypasses tab-under blocking which makes me worried. I can try to look into it.

Comment 7 by mustaq@chromium.org, Jan 16 2018

Cc: -csharrison@chromium.org
Components: UI>Browser>PopupBlocker
Labels: -Hotlist-Input-Dev
This behavior is similar to tab-under, and csharrison@chromium.org just informed me that tab-under-blocking doesn't block Link4 in the repro.

Anyway, he seems to be the correct owner for this.

I debugged this down, essentially it looks like:
1. The popup is going out first (expected), so gesture consumption won't stop this.
2. Chrome thinks the navigation to example.com starts in the foreground.

I guess a shift-click bypasses tab-under blocking because it isn't a synchronous window opening, so the subsequent navigation in the original tab is considered a foreground nav. We allow those mostly out of fear of breaking things. Maybe we can relax that though.
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 22 2018

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

commit 04fdc33d160b4a8d3a748e91e9bec8356339ab3d
Author: Charles Harrison <csharrison@chromium.org>
Date: Mon Jan 22 20:31:34 2018

[tab-under] Block tab-under navigations in the foreground

Previously, we allowed navigations to occur after a popup if they were
in the foreground, under the assumption that by the time a popup is
created, it will immediately be in the foreground. This is not the case
with shift-click navigations (or spoofed ones).

This will end up blocking more navigations, but we still allow navs
with a user gesture.

Bug:  733736 
Change-Id: I11e9ed928392de392fda71cab9498f002652fc7d
Reviewed-on: https://chromium-review.googlesource.com/871532
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530976}
[modify] https://crrev.com/04fdc33d160b4a8d3a748e91e9bec8356339ab3d/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/04fdc33d160b4a8d3a748e91e9bec8356339ab3d/chrome/browser/ui/blocked_content/tab_under_blocker_browsertest.cc
[modify] https://crrev.com/04fdc33d160b4a8d3a748e91e9bec8356339ab3d/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/04fdc33d160b4a8d3a748e91e9bec8356339ab3d/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h

Status: Fixed (was: Available)
Summary: Simulated shift-clicks bypass tab-under blocking (was: Chrome allows double navigation through simulated clicks)
Going to mark as fixed.
Blocking: 661629
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 3 2018

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

commit a6d156964fb0df04a99ccbea5e6d1c44c293697d
Author: Charlie Harrison <csharrison@chromium.org>
Date: Tue Apr 03 03:11:03 2018

[tab-under] Revert r530976 and bring back in-background constraint

This caused a spike in metrics tracking users allowing the redirect
via native UI. Addionally, it seems plausible that it breaks legitimate
auth cases (though I have no direct repro).

It is unfortunate that it allows the popup-redirect case, but that is
not an abused vector (yet), and we are trying to be conservative to
increase the chances of launch.

Bug:  733736 
Change-Id: I3559fa6d9e050e476da4c22346ea3ca81526bf06
Reviewed-on: https://chromium-review.googlesource.com/990996
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547610}
[modify] https://crrev.com/a6d156964fb0df04a99ccbea5e6d1c44c293697d/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/a6d156964fb0df04a99ccbea5e6d1c44c293697d/chrome/browser/ui/blocked_content/tab_under_blocker_browsertest.cc
[modify] https://crrev.com/a6d156964fb0df04a99ccbea5e6d1c44c293697d/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/a6d156964fb0df04a99ccbea5e6d1c44c293697d/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h

Status: WontFix (was: Fixed)
WontFixing :/

We can try to revisit after we end up shipping. Keeping the intervention applying on background / occluded pages only helps with predictability.

Sign in to add a comment