New issue
Advanced search Search tips

Issue 833048 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

[MacViews-Browser] Deleting an item in the Bookmarks Bar Folder Menu makes Chrome unresponsive

Project Member Reported by meh...@chromium.org, Apr 14 2018

Issue description

Chrome Version: Canary 68.0.3397.0
OS: macOS 10.12.6

What steps will reproduce the problem?
(1) Enable MacViews-Browser
(2) Right-click on a Folder on the Bookmarks Bar, so that the menu appears
(3) Right-click on an item, so that the context-menu appears
(4) Select "Delete", so that the item removes from the menu
(5) Now click somewhere else within Chrome to hide the menu or try to click on an item in the menu.

What is the expected result?
Chrome shouldn't be unresponsive.

What happens instead?
From now on you can't do anything within Chrome.

Workaround: Pressing the ESC-Key hides the folder menu and Chrome is responsive again.

A screencast is attached.

Thanks
Mehmet
 
screencast.mov
2.3 MB View Download

Comment 1 by meh...@chromium.org, Apr 14 2018

Summary: [MacViews-Browser] Deleting an item in the Bookmarks Bar Folder Menu makes Chrome unresponsive (was: [MacViews-Browser] Deleting an item in the Bookmarks Folder Menu makes Chrome unresponsive)
Cc: -ellyjo...@chromium.org
Labels: M-68 MacViews-Browser Sprint-1 Target-68
Owner: ellyjo...@chromium.org
Status: Assigned (was: Untriaged)
Labels: -Pri-2 Pri-1
Cc: gov...@chromium.org
Labels: Merge-Request-67
Status: Started (was: Assigned)
<https://chromium-review.googlesource.com/c/chromium/src/+/1012876>

We will want an M67 merge for this since it interferes quite a bit with using this menu - tagging for that.

Comment 5 by gov...@chromium.org, Apr 16 2018

Pls make sure change listed at #4 lands in trunk before 5:00 PM PT today and update the bug with canary result tomorrow. If change looks good in canary, I can approve merge to M67. Thank you.
#5: the change in #4 is still in review, so perhaps we won't be able to merge it to M67 after all. Oh well.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 17 2018

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

commit 37ab908e08c8a87602ec455705bad9aedde7a66e
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Apr 17 18:54:30 2018

macviews: reset closure animation on ReallyAccept

Accept() can be called more than once on the same MenuController, via context
menus; in this situation, it's important to clear the animation so that input
events get handled after the animation completes.

Bug:  833048 
Change-Id: Id3ac92ce91e78b57f2120d4527e35e597ea03067
Reviewed-on: https://chromium-review.googlesource.com/1012876
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551410}
[modify] https://crrev.com/37ab908e08c8a87602ec455705bad9aedde7a66e/ui/views/controls/menu/menu_controller.cc

Comment 8 by gov...@chromium.org, Apr 17 2018

Labels: -Merge-Request-67 Merge-Approved-67
Approving merge to M67 branch 3396 for cl listed at #7 without canary coverage per offline chat with ellyjones@ so we can run 50% experiment on Dev.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

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

commit 45f60154fefede0d91d64472422871fd7fa6d09a
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Apr 17 19:03:43 2018

macviews: reset closure animation on ReallyAccept

Accept() can be called more than once on the same MenuController, via context
menus; in this situation, it's important to clear the animation so that input
events get handled after the animation completes.

TBR=ellyjones@chromium.org

(cherry picked from commit 37ab908e08c8a87602ec455705bad9aedde7a66e)

Bug:  833048 
Change-Id: Id3ac92ce91e78b57f2120d4527e35e597ea03067
Reviewed-on: https://chromium-review.googlesource.com/1012876
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551410}
Reviewed-on: https://chromium-review.googlesource.com/1015547
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#55}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/45f60154fefede0d91d64472422871fd7fa6d09a/ui/views/controls/menu/menu_controller.cc

Status: Fixed (was: Started)
Labels: TE-Verified-68.0.3399.0 TE-Verified-M68
Able to reproduce the issue on chrome reported version 68.0.3397.0
Verified the fix on Mac 10.12.6 on Chrome version #68.0.3399.0 as per the comment#0
Attaching screen cast for reference.
Observed "Chrome is responsive"
Hence, the fix is working as expected.
Adding the verified label.

Thanks!
833048.mp4
1.1 MB View Download

Sign in to add a comment