Issue metadata
Sign in to add a comment
|
Regression: Focus ring is lost upon pressing tab on chrome://apps.
Reported by
aiman.an...@etouch.net,
Oct 26 2017
|
||||||||||||||||||||||
Issue descriptionChrome Version: 64.0.3250.0 (Official Build)b311d7bf3bc03d9706c0d43624afdb56a7978610- refs/heads/master@{#511680}(64-bit) OS: Mac(10.12.6). Steps to reproduce: 0. Ensure you have the All Controls tab access enabled in Mac (System Preferences >> Keyboard >> Shortcuts >> All controls) 1.Launch Chrome, go to chrome://apps, right click on Gmail App and select ‘Remove from chrome’ 2.On ‘Remove Gmail’ dialog box Press Tab and observe. Actual Result: Focus ring is lost. Expected Result: Focus should be in a loop. This is regression issue broken in ‘M-64’ and below per-revision bisect result Using the per-revision bisect providing the bisect results, Good Build: 64.0.3249.0(Revision: 511319). Bad Build: 64.0.3250.0(Revision: 511680). You are probably looking for a change made after 511672 (known good), but no later than 511673 (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/4ba0bf602b08a76355ca978bb6c19492729cfc54..b030a4a0884a87640ad65e51b20847c6213b8eab Suspect: https://chromium.googlesource.com/chromium/src/+/b030a4a0884a87640ad65e51b20847c6213b8eab @gab: 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: Issue is not seen on Win and Linux OS. Thank You!
,
Oct 31 2017
Note: Re-bisected this issue on different machine and getting the same culprit as mentioned. Suspect: https://chromium.googlesource.com/chromium/src/+/b030a4a0884a87640ad65e51b20847c6213b8eab The above issue is MAC specific.
,
Oct 31 2017
Ah ok my bad, didn't realize this was Mac specific. @robliao I'll need your help here to debug a Mac specific issue. What I think is happening is that in https://chromium-review.googlesource.com/c/chromium/src/+/594713 I erroneously made it such that NestableTasksAllowed() was true in system induced nested loops. I fixed that in https://chromium-review.googlesource.com/c/chromium/src/+/730864 However it looks like in the meantime someone made a change on the Mac side which should have required a MessageLoop::ScopedNestableTasksAllower prior to an induced system nested loop but didn't (because it worked without it, because of my bad change...). To prove this is the case you could try to sync to r511672 and revert r492263 (to show that it is a problem in the original impl as well and thus that the issue is a missing allowance per it not being required temporarily by my fault). If that proves to be true, need to debug to find the stack that requires a ScopedNestableTasksAllower. Thanks! PS: I Also just merged https://chromium-review.googlesource.com/c/chromium/src/+/730864 to M63 so it'll need to be fixed there as well. I expect the fix to be a one-liner when we find where the allowance needs to go so shouldn't be a big deal..
,
Oct 31 2017
,
Oct 31 2017
The per revision bisect indeed points to https://chromium.googlesource.com/chromium/src/+/b030a4a0884a87640ad65e51b20847c6213b8eab
,
Nov 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4383d025f03ee11649c1cd0501e93a2ab20e5286 commit 4383d025f03ee11649c1cd0501e93a2ab20e5286 Author: Robert Liao <robliao@chromium.org> Date: Wed Nov 01 21:10:54 2017 Add a ScopedNestableTaskAllower to Fix Tab Navigation MacOS's alert dialog seems to need allowing nestable tasks for tab navigation to work. BUG= 778562 Change-Id: Ic8654defa4e6ec1c7089639108e8a937e923b486 Reviewed-on: https://chromium-review.googlesource.com/747849 Commit-Queue: Robert Liao <robliao@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#513270} [modify] https://crrev.com/4383d025f03ee11649c1cd0501e93a2ab20e5286/chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm
,
Nov 1 2017
Pls request a merge to M63 branch 3239 once cl listed at #6 is baked/verified in Canary and safe to merge. Thank you.
,
Nov 2 2017
Note: Retested the above issue on latest canary #64.0.3256.0 on Mac(10.12.6) and fix is working as intended.
,
Nov 2 2017
>git tag --contains 4383d025f03ee11649c1cd0501e93a2ab20e5286 64.0.3256.0 64.0.3256.1 #8 has also verified the fix.
,
Nov 2 2017
Approving merge for CL listed at #6 based on comment #9.
,
Nov 2 2017
FYI: M63 merge has bee approved for https://bugs.chromium.org/p/chromium/issues/detail?id=778562&desc=2#c10.
,
Nov 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47894ca45a6ee9b64353976643640963551abdc2 commit 47894ca45a6ee9b64353976643640963551abdc2 Author: Robert Liao <robliao@chromium.org> Date: Thu Nov 02 19:58:30 2017 Add a ScopedNestableTaskAllower to Fix Tab Navigation MacOS's alert dialog seems to need allowing nestable tasks for tab navigation to work. BUG= 778562 TBR=gab@chromium.org,avi@chromium.org,tapted@chromium.org Change-Id: Ic8654defa4e6ec1c7089639108e8a937e923b486 Reviewed-on: https://chromium-review.googlesource.com/747849 Commit-Queue: Robert Liao <robliao@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#513270}(cherry picked from commit 4383d025f03ee11649c1cd0501e93a2ab20e5286) Reviewed-on: https://chromium-review.googlesource.com/752042 Reviewed-by: Robert Liao <robliao@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#353} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/47894ca45a6ee9b64353976643640963551abdc2/chrome/browser/ui/cocoa/extensions/extension_uninstall_dialog_cocoa.mm
,
Nov 3 2017
,
Nov 7 2017
Note: Retested the above issue on Beta # 63.0.3239.39 on Mac(10.12.6) and fix is working as intended.
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by gab@chromium.org
, Oct 30 2017Labels: Needs-Bisect
Owner: ----
Status: Untriaged (was: Assigned)