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

Issue 656948 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unable to open wrench menu after dragging bookmarked folder on to bookmarks bar.

Reported by vvishwak...@etouch.net, Oct 18 2016

Issue description

Version: 56.0.2894.0 (Official Build) canary 9e01f1257eb75274157fe7d246869b32d24b155b-refs/heads/master@{#425838} (32/64-bit)
OS: Windows (7,8,10)

What steps will reproduce the problem?
1) Launch chrome, press Cmd+Shift+B to enable Bookmarks bar.
2) Add a new folder on bookmarks bar and add another folder inside the added folder.
3) Click on added folder on bookmarks bar and drag the folder inside it on to the bookmarks bar.
4) Now click on wrench menu and observe.

Wrench menu does not open on clicking.

Wrench menu should open on clicking.

This is a Regression issue broken in M-56, will soon update other info
Manual bisect:
Good build: 56.0.2891.0
Bad build: 56.0.2894.0

Note: Issue is not seen on Mac OS, will update Linux info.
 
Actual.mp4
546 KB View Download
Expected.mp4
556 KB View Download
Labels: Needs-Bisect
Status: Untriaged (was: Unconfirmed)
Cc: rbasuvula@chromium.org
Labels: -hasbisect -Needs-Bisect hasbisect-per-revision OS-Linux
Owner: jonr...@chromium.org
Status: Assigned (was: Untriaged)
Using the per-revision bisect providing the bisect results,
Good build: 56.0.2891.0 (Revision: 425529).
Bad build: 56.0.2894.0 (Revision: 425838).

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

CHANGE-LOG URL:
---------------------------------------
https://chromium.googlesource.com/chromium/src/+log/affff11fa9faa078bcf269159ec1b707d05c5353..24470df35ac9d98824f447d9a0112c0bdbdf1cb5


From the CL above, assigning the issue to the concern owner

@ jonross : 
------------------
Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Review-Url: https://codereview.chromium.org/2422283002

Note : Able to reproduce the issue in linux ubuntu 14.04.
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 20 2016

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

commit 6f08e5ae5e67d68510072d6873366a06841064fe
Author: jonross <jonross@chromium.org>
Date: Thu Oct 20 20:59:43 2016

Fix opening App Menu after Bookmarks Drag

After dragging a nested bookmarks folder to the bookmarks bar, you could not
open the App Menu.

During a Drag operation BookmarkBarView can call Cancel in several
circumstances. If a Cancel was received by MenuController before
OnDragCompleted. Then the internal call to Cancel would not properly teardown
the menu. With an active, but hidden, menu new menus, such as the AppMenu, could
not launch.

This change updates the logic of MenuController::OnDragCompleted to make sure
that teardown occurs.

TEST=MenuControllerTest.AsynchronousDragComplete, MenuControllerTest.AsynchronousCancelDuringDrag
BUG= 656948 

Review-Url: https://chromiumcodereview.appspot.com/2433933007
Cr-Commit-Position: refs/heads/master@{#426592}

[modify] https://crrev.com/6f08e5ae5e67d68510072d6873366a06841064fe/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/6f08e5ae5e67d68510072d6873366a06841064fe/ui/views/controls/menu/menu_controller_unittest.cc

Labels: Merge-Request-55
Status: Fixed (was: Started)
I just landed the fix for this: https://chromiumcodereview.appspot.com/2433933007/

This was caused by https://codereview.chromium.org/2422283002 as noted in #2

as that is merged into M55 I'm requesting a merge for this.
Labels: Merge-Request-54
As the source of this ( issue 636397 ) landed in 54, there was a request to merge the fixes in case of a future update.

Regardless we'd still want this in M55

Comment 7 by dimu@google.com, Oct 24 2016

Labels: -Merge-Request-54 Merge-Review-54 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

Comment 8 by dimu@google.com, Oct 24 2016

Labels: -Merge-Request-55 Merge-Review-55
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

Comment 9 by gov...@chromium.org, Oct 24 2016

Before we approve merge to M55 branch 2883, could you please confirm whether this change is baked/verified in Canary and safe to merge to M55?
I just verified this change on Linux 56.0.2897.0 (Official Build) dev (64-bit)

It is safe to merge into M55

Comment 11 by dimu@google.com, Oct 24 2016

Labels: -Hotlist-Merge-review -Merge-Review-54 -Merge-Review-55 Merge-Request-54 Merge-Request-55

Comment 12 by dimu@chromium.org, Oct 24 2016

Labels: -Merge-Request-54 Merge-Review-54 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M54), manual review required.

Comment 13 by dimu@chromium.org, Oct 24 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)

Comment 14 by dimu@chromium.org, Oct 24 2016

Labels: -Merge-Request-54 Merge-Review-54 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M54), manual review required.
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 24 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa

commit 54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa
Author: jonross <jonross@chromium.org>
Date: Mon Oct 24 22:00:41 2016

Merge Fix opening App Menu after Bookmarks Drag
Fix opening App Menu after Bookmarks Drag

After dragging a nested bookmarks folder to the bookmarks bar, you could not
open the App Menu.

During a Drag operation BookmarkBarView can call Cancel in several
circumstances. If a Cancel was received by MenuController before
OnDragCompleted. Then the internal call to Cancel would not properly teardown
the menu. With an active, but hidden, menu new menus, such as the AppMenu, could
not launch.

This change updates the logic of MenuController::OnDragCompleted to make sure
that teardown occurs.

TBR=sky@chromium.org
NOTRY=true
NOPRESUBMIT=true
TEST=MenuControllerTest.AsynchronousDragComplete, MenuControllerTest.AsynchronousCancelDuringDrag
BUG= 656948 

Review-Url: https://chromiumcodereview.appspot.com/2433933007
Cr-Commit-Position: refs/heads/master@{#426592}
(cherry picked from commit 6f08e5ae5e67d68510072d6873366a06841064fe)

Review-Url: https://codereview.chromium.org/2448673002
Cr-Commit-Position: refs/branch-heads/2883@{#268}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa/ui/views/controls/menu/menu_controller_unittest.cc

Labels: -Merge-Review-54 Merge-Rejected-54
This is too late for M54, we already shipped the last stable refresh 54.0.2840.71, barring a critical regression.
Cc: hdodda@chromium.org
Labels: TE-Verified-M55 TE-Verified-55.0.2883.28
Tested the issue on Windows 10 , Ubuntu 14.04 using chrome beta version #55.0.2883.28.

Observed that fix is working as expected. Attached screencast for reference.

Adding the TE-Verified labels.

Thanks!
656948.ogv
3.1 MB View Download
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa

commit 54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa
Author: jonross <jonross@chromium.org>
Date: Mon Oct 24 22:00:41 2016

Merge Fix opening App Menu after Bookmarks Drag
Fix opening App Menu after Bookmarks Drag

After dragging a nested bookmarks folder to the bookmarks bar, you could not
open the App Menu.

During a Drag operation BookmarkBarView can call Cancel in several
circumstances. If a Cancel was received by MenuController before
OnDragCompleted. Then the internal call to Cancel would not properly teardown
the menu. With an active, but hidden, menu new menus, such as the AppMenu, could
not launch.

This change updates the logic of MenuController::OnDragCompleted to make sure
that teardown occurs.

TBR=sky@chromium.org
NOTRY=true
NOPRESUBMIT=true
TEST=MenuControllerTest.AsynchronousDragComplete, MenuControllerTest.AsynchronousCancelDuringDrag
BUG= 656948 

Review-Url: https://chromiumcodereview.appspot.com/2433933007
Cr-Commit-Position: refs/heads/master@{#426592}
(cherry picked from commit 6f08e5ae5e67d68510072d6873366a06841064fe)

Review-Url: https://codereview.chromium.org/2448673002
Cr-Commit-Position: refs/branch-heads/2883@{#268}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa/ui/views/controls/menu/menu_controller.cc
[modify] https://crrev.com/54c8ac002182ab32f3c931f35aea1ed6e2a0c3fa/ui/views/controls/menu/menu_controller_unittest.cc

Comment 19 by dimu@google.com, Nov 4 2016

[Automated comment] removing mislabelled merge-merged-2840

Comment 20 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment