New issue
Advanced search Search tips

Issue 908681 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Auto-hide shelf should stay visible after app tap that opens menu

Project Member Reported by mccanny@chromium.org, Nov 27

Issue description

Chrome OS version: 72.0.3609.3

Shelf should stay visible after app icon tap that brings up window disambiguation menu. Currently the menu appears, but the shelf disappears, leading the menu to float awkwardly. See screencap: https://drive.google.com/open?id=1ynyDg-EVnc9spirqsCpaGg_wdEabaP-5
 
Labels: OS-Chrome
Ben, feel this should has been fixed in  issue 905847 . The fix was landed in 72.0.3617.0. Let me know if you can still see it after that. Thanks.


Oh, thanks Min. I can see it in 72.0.3618, but I'm not sure I see a fix for the other bug either. Maybe I'm not understanding that issue.
Hi Ben, please take a look of the recorded video here https://drive.google.com/file/d/0B5I0jFeLxqIiU2FnX0g3TTMwaUNRRjV2THFzRnVFQWNuWWpN/view?usp=sharing

version: 72.0.3263.0

It includes two actions,
1. Long press the shelf item in auto-hide shelf to open the context menu.
2. Drag the shelf item in auto-hide shelf after context menu is opened.
Labels: -Pri-3 M-72 Pri-1
This seems like a P-1 to me, IIUC:

1. Have two chrome windows open.
2. Set shelf to autohide.
3. Tap the chrome icon to show disambiguation menu.
4. *Shelf hides* while the menu is shown.
Alex, Min couldn't repro, so just waiting on canary to see if her change resolves it.
Status: Started (was: Untriaged)
Ben, I can repro it through the steps Alex provided in #4. The browser have two types context menu, one is context menu we used to open new window or close, another is the menu when you opened multiple tabs used to switch between the multiple tabs.
Thanks Alex. I am going to take a look of this.
Thanks Min!
Labels: -M-72 -m-72 M-73
Bulk moving <p-1's to the next milestone because we branched to M-73.
Labels: -M-73 M-72
(didn't mean to grab these P-1's)
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 3

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

commit 2be72dc0fe4e7617da33ba6ae7905d8b33cfb992
Author: Min Chen <minch@google.com>
Date: Mon Dec 03 23:37:19 2018

shelf: Keep auto-hide shelf shown if menu is shown.

Open the application select menu of chrome shortcut will trigger the
window stacking change. It will update the shelf visibility before
showing the menu. Call UpdateVisibilityState after the menu is shown
to keep the auto-hide shelf shown.

Bug:  908681 
Change-Id: I1e4dca91a03ca85df5ddb1511bd2b4f3fe6c8d63
Reviewed-on: https://chromium-review.googlesource.com/c/1356595
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613320}
[modify] https://crrev.com/2be72dc0fe4e7617da33ba6ae7905d8b33cfb992/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/2be72dc0fe4e7617da33ba6ae7905d8b33cfb992/ash/shelf/shelf_view.cc
[modify] https://crrev.com/2be72dc0fe4e7617da33ba6ae7905d8b33cfb992/ash/shelf/shelf_view.h
[modify] https://crrev.com/2be72dc0fe4e7617da33ba6ae7905d8b33cfb992/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/2be72dc0fe4e7617da33ba6ae7905d8b33cfb992/ash/shelf/shelf_widget.h

Labels: Merge-Request-72
Labels: Merge-Approved-72
Project Member

Comment 13 by sheriffbot@chromium.org, Dec 4

Labels: -Merge-Request-72 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 14 by bugdroid1@chromium.org, Dec 5

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d1870b8488853c226697a64e72d08df550cd5cd

commit 0d1870b8488853c226697a64e72d08df550cd5cd
Author: Min Chen <minch@google.com>
Date: Wed Dec 05 00:39:07 2018

[Merge to M72]shelf: Keep auto-hide shelf shown if menu is shown.

TBR=minch@google.com

Open the application select menu of chrome shortcut will trigger the
window stacking change. It will update the shelf visibility before
showing the menu. Call UpdateVisibilityState after the menu is shown
to keep the auto-hide shelf shown.

(cherry picked from commit 2be72dc0fe4e7617da33ba6ae7905d8b33cfb992)

Bug:  908681 
Change-Id: I1e4dca91a03ca85df5ddb1511bd2b4f3fe6c8d63
Reviewed-on: https://chromium-review.googlesource.com/c/1356595
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613320}
Reviewed-on: https://chromium-review.googlesource.com/c/1362456
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#54}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/0d1870b8488853c226697a64e72d08df550cd5cd/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/0d1870b8488853c226697a64e72d08df550cd5cd/ash/shelf/shelf_view.cc
[modify] https://crrev.com/0d1870b8488853c226697a64e72d08df550cd5cd/ash/shelf/shelf_view.h
[modify] https://crrev.com/0d1870b8488853c226697a64e72d08df550cd5cd/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/0d1870b8488853c226697a64e72d08df550cd5cd/ash/shelf/shelf_widget.h

Status: Fixed (was: Started)
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 0d1870b8488853c226697a64e72d08df550cd5cd was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/0d1870b8488853c226697a64e72d08df550cd5cd

Commit: 0d1870b8488853c226697a64e72d08df550cd5cd
Author: minch@google.com
Commiter: minch@chromium.org
Date: 2018-12-05 00:39:07 +0000 UTC

[Merge to M72]shelf: Keep auto-hide shelf shown if menu is shown.

TBR=minch@google.com

Open the application select menu of chrome shortcut will trigger the
window stacking change. It will update the shelf visibility before
showing the menu. Call UpdateVisibilityState after the menu is shown
to keep the auto-hide shelf shown.

(cherry picked from commit 2be72dc0fe4e7617da33ba6ae7905d8b33cfb992)

Bug:  908681 
Change-Id: I1e4dca91a03ca85df5ddb1511bd2b4f3fe6c8d63
Reviewed-on: https://chromium-review.googlesource.com/c/1356595
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613320}
Reviewed-on: https://chromium-review.googlesource.com/c/1362456
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#54}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment