New issue
Advanced search Search tips

Issue 839303 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-14
OS: Windows , Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Toggle button doesn't turn ON/OFF after tap touch on "Allow in incognito" text

Reported by khushal....@etouch.net, May 3 2018

Issue description

Chrome Version: 68.0.3418.0 (Official Build) Revision	5319d9b60c08fee2ca45593c7a67688408ce5d73-refs/heads/master@{#555651} (32/64 bit)
OS: Windows 10 (Touch device)

What steps will reproduce the problem?
(1) Launch chrome, navigate to chrome://extensions/ page and touch on "Details" of any extension.
(2) Now try to tap touch on "Allow in incognito" text so as to turn toggle ON/OFF and observe.

Actual Result: Toggle button doesn't turn ON/OFF after tap touch on "Allow in incognito" text.

Expected Result: Toggle button should turn ON/OFF after tap touch on "Allow in incognito" text.

This is a regression issue, broken in M-67 series, and providing bisect using per-bisect revision:

Good Build: 67.0.3381.0 (Revision:545919)
Bad Build:  67.0.3382.0 (Revision:546346)

You are probably looking for a change made after 546102 (known good), but no later than 546103 (first known bad).

CHANGE-LOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/2b62de0bf01e7c71c82719ec30e19a32ca251ad6..12f9cbe62bb8f6f09782c1ac05bff57bd61f4d51

Suspect: https://chromium.googlesource.com/chromium/src/+/bda1898036dc3ddac9e4d5f9df2f76f68b072d51

@nzolghadr: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note:
1) Issue is Touch specific and it is not seen on Click event.
2) Issue is also seen on M-67 Beta (build #67.0.3396.30) and M-68 Dev (build #68.0.3409.2).
3) Issue is not seen on Linux (14.04 LTS) and Mac (10.12.6, 10.13.1, 10.13.5) OS.

Kindly refer attached screen cast.

Thank You..!!
 
Actual Video.mp4
663 KB View Download
Expected Video.mp4
465 KB View Download
Labels: ReleaseBlock-Stable OS-iOS
marking as RBS, please change if required.
Status: Fixed (was: Assigned)
I just tested this on a Win 10 surface with Canary 68.0.3423.0 and it seems working with touch and I was able to toggle on/off that option. Let me know if you are still able to reproduce this issue.
Components: Blink>Input
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-67; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-67 label, otherwise remove Merge-TBD label. Thanks.
 nzolghadr@, per comment #0, this is regressed in M67. So please request a merge to M67 for the fix. Thank you.
There was no fix. I mentioned that I wasn't able to reproduce the issue at all. I also just tested with 67.0.3396.30 and the same result. Everything works fine.
Aside from that whatever it was it is unlikely that is caused by my change as my change only changes mouse path. So that might be something else that someone else has already taken care.
Cc: pbomm...@chromium.org
Labels: -Merge-TBD
Thank you  nzolghadr@.

+pbommana@, could you ptal (bisect again) and reassign to appropriate dev?
I am unable to reproduce the issue on my Windows 10 machines touch devices on latest Chrome canary and Beta i.e., 68.0.3423.1 and 67.0.3396.30.


With respect to comment #6 & 8,

Retested the issue on latest canary build #68.0.3424.0 and reported build #68.0.3418.0, issue is still reproducible while tap touch on "Allow in incognito" text. Also, re-bisected the issue and still found same suspect.


Actual Video_68.0.3424.0.mp4
574 KB View Download
Status: Started (was: Fixed)
I now can reproduce the issue. For the record it does matter where you tap. The tap on the toggle button itself works fine. But the tap on other parts of that row stopped working.
I'm working on a fix.
Project Member

Comment 11 by bugdroid1@chromium.org, May 10 2018

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

commit 2306a9caf973ef319f5a53356568434159643675
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Thu May 10 20:39:42 2018

Update modifiers when buttons is set in MouseEvent

NACL expects modifiers to contain buttons information.
So we need to also set that information not only in
buttons field but also modifiers.

Bug:  839539 ,  839303 
Change-Id: Ib859a2e071984be4625f0a42094982c7513ae5ae
Reviewed-on: https://chromium-review.googlesource.com/1052171
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ella Ge <eirage@chromium.org>
Commit-Queue: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557657}
[modify] https://crrev.com/2306a9caf973ef319f5a53356568434159643675/third_party/blink/renderer/core/events/mouse_event.cc
[modify] https://crrev.com/2306a9caf973ef319f5a53356568434159643675/third_party/blink/renderer/core/exported/web_plugin_container_test.cc

NextAction: 2018-05-11
Thank you nzolghadr@. Pls update the bug with canary result tomorrow. 
*** Bulk Edit ***
M67 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. 

If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
The NextAction date has arrived: 2018-05-11
Labels: TE-Verified-M68 TE-Verified-68.0.3427.0
Rechecked the above issue on latest Canary build #68.0.3427.0 for Windows 10 OS (Touch device) and the issue is fixed.

Please refer the attached screen-cast.
Fixed Video.mp4
735 KB View Download
Labels: -OS-iOS OS-Chrome
Changing OS from iOS to CrOS. 
Labels: Merge-Request-67
How safe is the Cl listed at #11 to merge?
It is a minimal change really. I'd say pretty safe. That CL addresses both this issue and  issue 839539 . I was wondering if one of the tester can also verify the other one before me merging it to the branch.
pbommana@, could you pls verify  issue 839539  on latest canary? Thank you.
Verified  issue#839539  on latest Chrome Canary i.e., 68.0.3427.0 on Windows, Linux and Mac(by nzolghadr), fix looks good.
Can you also comment on the original bug for the record. Thanks
Labels: -Merge-Request-67 Merge-Approved-67
Thank you pbommana@ and nzolghadr@.

Approving merge to M67 branch 3396 based on comment #15, #19 and #21. Pls merge ASAP and mark bug as fixed if nothing else is pending. 
NextAction: 2018-05-14
Pls merge on Monday morning. Thank you.
The NextAction date has arrived: 2018-05-14
Project Member

Comment 26 by bugdroid1@chromium.org, May 14 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14ee3910944246842c440e3ad910a7279e24d28a

commit 14ee3910944246842c440e3ad910a7279e24d28a
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Mon May 14 14:49:47 2018

Update modifiers when buttons is set in MouseEvent

NACL expects modifiers to contain buttons information.
So we need to also set that information not only in
buttons field but also modifiers.

Bug:  839539 ,  839303 
Change-Id: Ib859a2e071984be4625f0a42094982c7513ae5ae
Reviewed-on: https://chromium-review.googlesource.com/1052171
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ella Ge <eirage@chromium.org>
Commit-Queue: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#557657}(cherry picked from commit 2306a9caf973ef319f5a53356568434159643675)
Reviewed-on: https://chromium-review.googlesource.com/1057688
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#585}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/14ee3910944246842c440e3ad910a7279e24d28a/third_party/blink/renderer/core/events/mouse_event.cc
[modify] https://crrev.com/14ee3910944246842c440e3ad910a7279e24d28a/third_party/blink/renderer/core/exported/web_plugin_container_test.cc

Status: Fixed (was: Started)

Sign in to add a comment