New issue
Advanced search Search tips

Issue 701599 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-03-21
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Failed to open links on "CUSTOMERS ALSO VIEWED' area - bloomingdales.com

Project Member Reported by songsuk@chromium.org, Mar 15 2017

Issue description

Chrome Version       : 58.0.3029.18 (Official Build) dev (64-bit)
Platform             : ChromeOS 9334.10.0 - pain

What steps will reproduce the problem?
(1)  navigate to the following link http://www1.bloomingdales.com/shop/product/coach-1941-glovetanned-pebble-rogue-satchel?ID=1805297&CategoryID=1004772&brandIndex=1#fn=ppp%3D%26spp%3D2%26sp%3D1%26rid%3D113|BOOST%20SAVED%20SET%26spc%3D122%26rsid%3Dundefined%26pn%3D1|2|2|122
(2) click on links/images on "CUSTOMERS ALSO VIEWED" area


What is the expected result?
The link/image pages should be opened 


What happens instead?
Nothing happens. 

Please provide any additional information below. Attach a screenshot if
possible.

Not reproduce the issue on Chrome 57.0.2987.100/CrOS 9202.51.0 - Candy

Able to reproduce the issue on 58.0.3029.14- Ubuntu14.04, 
59.0.3041.0 canary-Mac, 59.0.3041.1-canary/win7






 

Comment 1 by ajha@chromium.org, Mar 15 2017

Cc: bokan@chromium.org ajha@chromium.org
Components: Blink>Input
Labels: -Needs-Bisect hasbisect-per-revision
Owner: nzolghadr@chromium.org
Status: Assigned (was: Untriaged)
Able to reproduce the issue on the latest canary(59.0.3042.0) on Windows-10, Mac OS 10.12.3 and Linux Ubuntu 14.04.  

Regressed in M-58:
==================
Last good build: 58.0.3026.0
First bad build: 58.0.3027.0

Changelog:
==========
https://chromium.googlesource.com/chromium/src/+log/ef2a8e07616635ec90e8b30a95fb8a43a7f0ec9a..14cea387ce9359e5f9fe3e90c996adf75e092e64

nzolghadr@: Could you please take a look at this.

Thank you!

Comment 2 by tkent@chromium.org, Mar 16 2017

Components: -Blink>HTML>Link
Labels: Needs-triage-Mobile
NextAction: 2017-03-21
This is a regression in M58 and tagged as RBS. Please try to resolve ASAP. 
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 20 2017

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

commit ddb951c0cc2912acd57e4238677b82e6360ac1ad
Author: nzolghadr <nzolghadr@chromium.org>
Date: Mon Mar 20 16:45:46 2017

Ignore pointer capture target while calculating click target

After https://codereview.chromium.org/2681443003
it turns out we are creating some regressions.
So this change keeps the essence of the original
change for the purpose of keeping the metric but
ignores the target that is calculated from the
captured target and use the original click target
regardless of the capturing.

BUG= 701599 ,  699933 

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

[add] https://crrev.com/ddb951c0cc2912acd57e4238677b82e6360ac1ad/third_party/WebKit/LayoutTests/external/wpt/pointerevents/pointerevent_click_during_capture-manual-expected.txt
[modify] https://crrev.com/ddb951c0cc2912acd57e4238677b82e6360ac1ad/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/ddb951c0cc2912acd57e4238677b82e6360ac1ad/third_party/WebKit/Source/core/input/PointerEventManager.cpp

Labels: Merge-Request-58
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 21 2017

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

commit 38052ae353aee8c943b570b5a2c10be82a48c1ae
Author: johnme <johnme@chromium.org>
Date: Tue Mar 21 12:45:46 2017

Revert of Ignore pointer capture target while calculating click target (patchset #1 id:1 of https://codereview.chromium.org/2755753003/ )

Reason for revert:
Tentative revert. Hit testing broke in the following tests, and this patch looks the most closely related:

AccessibilityHitTestingBrowserTest.HitTestingInIframes
AccessibilityHitTestingBrowserTest.CachingAsyncHitTestingInIframes

Original issue's description:
> Ignore pointer capture target while calculating click target
>
> After https://codereview.chromium.org/2681443003
> it turns out we are creating some regressions.
> So this change keeps the essence of the original
> change for the purpose of keeping the metric but
> ignores the target that is calculated from the
> captured target and use the original click target
> regardless of the capturing.
>
> BUG= 701599 ,  699933 
>
> Review-Url: https://codereview.chromium.org/2755753003
> Cr-Commit-Position: refs/heads/master@{#458091}
> Committed: https://chromium.googlesource.com/chromium/src/+/ddb951c0cc2912acd57e4238677b82e6360ac1ad

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org,nzolghadr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 701599 ,  699933 

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

[delete] https://crrev.com/c5521157c6a33f93c544d45f5db15dbbe14ea8cd/third_party/WebKit/LayoutTests/external/wpt/pointerevents/pointerevent_click_during_capture-manual-expected.txt
[modify] https://crrev.com/38052ae353aee8c943b570b5a2c10be82a48c1ae/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/38052ae353aee8c943b570b5a2c10be82a48c1ae/third_party/WebKit/Source/core/input/PointerEventManager.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 21 2017

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

commit 4b4080856b160e1abf3c3a21cacdd165847d90d8
Author: johnme <johnme@chromium.org>
Date: Tue Mar 21 16:23:37 2017

Reland of Ignore pointer capture target while calculating click target (patchset #1 id:1 of https://codereview.chromium.org/2764813004/ )

Reason for revert:
Reverting didn't fix
https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Tester/builds/7242
so this patch seems innocent.

Original issue's description:
> Revert of Ignore pointer capture target while calculating click target (patchset #1 id:1 of https://codereview.chromium.org/2755753003/ )
>
> Reason for revert:
> Tentative revert. Hit testing broke in the following tests, and this patch looks the most closely related:
>
> AccessibilityHitTestingBrowserTest.HitTestingInIframes
> AccessibilityHitTestingBrowserTest.CachingAsyncHitTestingInIframes
>
> Original issue's description:
> > Ignore pointer capture target while calculating click target
> >
> > After https://codereview.chromium.org/2681443003
> > it turns out we are creating some regressions.
> > So this change keeps the essence of the original
> > change for the purpose of keeping the metric but
> > ignores the target that is calculated from the
> > captured target and use the original click target
> > regardless of the capturing.
> >
> > BUG= 701599 ,  699933 
> >
> > Review-Url: https://codereview.chromium.org/2755753003
> > Cr-Commit-Position: refs/heads/master@{#458091}
> > Committed: https://chromium.googlesource.com/chromium/src/+/ddb951c0cc2912acd57e4238677b82e6360ac1ad
>
> TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org,nzolghadr@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 701599 ,  699933 
>
> Review-Url: https://codereview.chromium.org/2764813004
> Cr-Commit-Position: refs/heads/master@{#458382}
> Committed: https://chromium.googlesource.com/chromium/src/+/38052ae353aee8c943b570b5a2c10be82a48c1ae

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org,nzolghadr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 701599 ,  699933 

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

[add] https://crrev.com/4b4080856b160e1abf3c3a21cacdd165847d90d8/third_party/WebKit/LayoutTests/external/wpt/pointerevents/pointerevent_click_during_capture-manual-expected.txt
[modify] https://crrev.com/4b4080856b160e1abf3c3a21cacdd165847d90d8/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/4b4080856b160e1abf3c3a21cacdd165847d90d8/third_party/WebKit/Source/core/input/PointerEventManager.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 21 2017

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

commit 1b708802860053bad35e8eaad2508f87701ec18b
Author: nzolghadr <nzolghadr@chromium.org>
Date: Tue Mar 21 19:22:57 2017

Revert of Ignore pointer capture target while calculating click target (patchset #1 id:1 of https://codereview.chromium.org/2755753003/ )

Reason for revert:
Reverting all the changes related to the click target in capturing as they are splitted across multiple commits.

Original issue's description:
> Ignore pointer capture target while calculating click target
>
> After https://codereview.chromium.org/2681443003
> it turns out we are creating some regressions.
> So this change keeps the essence of the original
> change for the purpose of keeping the metric but
> ignores the target that is calculated from the
> captured target and use the original click target
> regardless of the capturing.
>
> BUG= 701599 ,  699933 
>
> Review-Url: https://codereview.chromium.org/2755753003
> Cr-Commit-Position: refs/heads/master@{#458091}
> Committed: https://chromium.googlesource.com/chromium/src/+/ddb951c0cc2912acd57e4238677b82e6360ac1ad

TBR=dtapuska@chromium.org,mustaq@chromium.org,bokan@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 701599 ,  699933 

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

[delete] https://crrev.com/5de6f5cb0ef513f204df00f0d13d4da3f6a0fd86/third_party/WebKit/LayoutTests/external/wpt/pointerevents/pointerevent_click_during_capture-manual-expected.txt
[modify] https://crrev.com/1b708802860053bad35e8eaad2508f87701ec18b/third_party/WebKit/Source/core/input/MouseEventManager.cpp
[modify] https://crrev.com/1b708802860053bad35e8eaad2508f87701ec18b/third_party/WebKit/Source/core/input/PointerEventManager.cpp

Comment 10 by ajha@chromium.org, Mar 22 2017

Just to update the behavior of this wrt 'Needs-triage-Mobile' label added in C#3.

Tested this on:
================ 
Chrome version: latest M-58 Beta (58.0.3029.21)
Devices tested: Samsung Galaxy Tab3 & Nexus5 Handset. 
Platform tested:(Android 4.4.2; GT-P5210 Build/KOT49H) & (Android version: 7.0.99/MRA20)   

Issue is not reproducible on Android as per the above test on Tablet and Handset.

Project Member

Comment 11 by sheriffbot@chromium.org, Mar 22 2017

Labels: -Merge-Request-58 Merge-Review-58 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Merge-Request-58
Project Member

Comment 13 by sheriffbot@chromium.org, Mar 22 2017

Labels: -Merge-Request-58
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Has this been tested on Canary, and do we have enough automatic test coverage for this change? Please let us know before we approve merge. 
Yes. It has been tested in Canary. This is a revert from the previous patch I landed along with with its test not a new patch.
Cc: gov...@chromium.org
govind@ I need to merge this change to the branch. Is there anything else I need to do here to get the approval?
Labels: -Merge-Review-58 Merge-Approved-58
Approving merge to M58 branch 3029 based on comment #15. Please merge ASAP. Thank you.
Is that okay to do the merge on Monday? I'm a little paranoid for merging stuff on Friday!
Sure, Monday is fine. Thank you.
Your change is approved for M58. Please ensure that fix works in canary. If all looks good, merge ASAP so that it will be picked up for next Beta Release, RC cut on (Tuesday-03/28) at 4.00 PM PST.
Status: Fixed (was: Started)
This got reverted in the branch as part of  issue 699933 .
Labels: -Merge-Approved-58 merge-merged-3029

Sign in to add a comment