Extension icons disappear when pushed to dotted menu |
||||||||||||||
Issue descriptionChrome Version: Version 69.0.3486.0 (Official Build) canary (64-bit) OS: Mac OS X 10.13.5 What steps will reproduce the problem? (1) Add multiple Chrome extensions (2) Display them beside omnibar (3) Push them onto menu bar by dragging left edge of left most extension to the right What is the expected result? See all hidden installed and active Chrome extensions in the menu. What happens instead? Only see couple of them in the menu.
,
Jul 11
Able to reproduce the issue on reported version 69.0.3486.0 and latest chrome 69.0.3488.0 using Mac 10.13.5, hence providing Bisect Info Note: Issue is not seen on Linux and Windows Bisect Info: ================ Good build: 69.0.3456.0 Bad build: 69.0.3457.0 You are probably looking for a change made after 566524 (known good), but no later than 566525 (first known bad). https://chromium.googlesource.com/chromium/src/+log/843f4c596964746062da6786e6d4baac53720b3c..66e935ac491542bde00638e1484ca2c5fdbbf084 Reviewed-on: https://chromium-review.googlesource.com/1091497 @Patrick Monette: Please confirm the issue and help in re-assigning if it is not related to your change. Adding ReleaseBlock-Beta as it is seems a recent break, feel free to remove it if not applicable. Thanks!
,
Jul 11
+cc lgrey@ - looking at the linked CL it seems odd that this would have broken only on Mac. Does this seem plausible to you?
,
Jul 11
Maybe, if it throws something out of whack with toolbar layout. The original report is from Cocoa browser; viswa.karala@ are we sure bisect was Cocoa (#views-browser-windows disabled) the whole way?
,
Jul 11
M69 branch is coming VERY soon on July 19th, Your bug is marked as ReleaseBlock-Beta for M69. Please try to land the fix ASAP to trunk in order to prevent many merges going after M69 branch. This will also help us to branch M69 from high quality trunk. Thank you.
,
Jul 16
Friendly ping to get an update as it is marked as RBB. Thanks..!
,
Jul 16
Mac triage: needs feedback from reporter per #4
,
Jul 17
M69 branch is coming VERY soon on this Thursday, July 19th, Your bug is marked as ReleaseBlock-Beta for M69. Please try to land the fix ASAP to trunk in order to prevent many merges going after M69 branch. This will also help us to branch M69 from high quality trunk. Thank you.
,
Jul 17
Is there anyone knowledgeable with extension icons that can take this bug? My CL should not have changed anything on Mac given that this was a Windows-only feature. I am not familiar with the code and thus can't really debug this issue.
,
Jul 19
Could some one from Extensions team please take a look into this as per C#9 & it is marked as RBB. Thanks..!
,
Jul 19
Readding Needs-Bisect due to c#4
,
Jul 20
Tested the issue on chrome reported version# 69.0.3486.0 using Mac 10.12.6 and Mac 10.13.5 got same bisect range and change-log as mentioned in comment# 2. Good build: 69.0.3456.0 Bad build: 69.0.3457.0 Also tested the issue on Ubuntu 14.04 and Windows-10, unable to reproduce the issue there. Able to reproduce the issue on mac by making the flag #views-browser-windows Default and Disable, but on Enabling the flag #views-browser-windows, we are unable to reproduce the issue. Hence removing Needs-Bisect label. Thanks!
,
Jul 20
Thanks for bisecting again. I think I've figured out the outline of what's going on here.
,
Jul 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/86341923722bec89a92a681b582d5e465c690541 commit 86341923722bec89a92a681b582d5e465c690541 Author: Leonard Grey <lgrey@chromium.org> Date: Fri Jul 20 17:00:42 2018 Cocoa: ensure app menu makes space from browser action overflow container https://chromium-review.googlesource.com/c/chromium/src/+/1091497 removed an item from the app menu that never appeared on Mac. Unfortunately, even though the item was never *displayed*, it appears to have factored into NSMenu's calculation for what its max size should be. When the item was removed, the max size is no longer able to account for multiple rows in the browser action overflow container, so all rows but the last are clipped. This change removes and readds the extension overflow container when it's finished sizing itself to prompt the menu to reevaluate its height. We can't do this before we add the item to the menu because the menu's width factors into the overflow container's size calculation. It seems like there should be a less awkward way to do this, but I can't find it. Bug: 862005 Change-Id: Ib17ed810855041226786b16ca94f0a60d88dc892 Reviewed-on: https://chromium-review.googlesource.com/1145133 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Leonard Grey <lgrey@chromium.org> Cr-Commit-Position: refs/heads/master@{#576905} [modify] https://crrev.com/86341923722bec89a92a681b582d5e465c690541/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
,
Jul 20
,
Jul 20
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; 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-69 label, otherwise remove Merge-TBD label. Thanks.
,
Jul 20
,
Jul 21
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 23
Able to reproduce the issue on chrome reported version 69.0.3486.0(Build without fix) Verified the fix on Mac 10.12.6 on Chrome version #70.0.3500.0 as per the comment#0 Attaching screenshot for reference. Observed "able to see all hidden installed and active Chrome extensions in the menu" Hence, the fix is working as expected. Adding the verified label. Thanks!
,
Jul 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3524fc6789095cfe23bee396f26e7c458033552 commit d3524fc6789095cfe23bee396f26e7c458033552 Author: Leonard Grey <lgrey@chromium.org> Date: Mon Jul 23 13:47:58 2018 Cocoa: ensure app menu makes space from browser action overflow container https://chromium-review.googlesource.com/c/chromium/src/+/1091497 removed an item from the app menu that never appeared on Mac. Unfortunately, even though the item was never *displayed*, it appears to have factored into NSMenu's calculation for what its max size should be. When the item was removed, the max size is no longer able to account for multiple rows in the browser action overflow container, so all rows but the last are clipped. This change removes and readds the extension overflow container when it's finished sizing itself to prompt the menu to reevaluate its height. We can't do this before we add the item to the menu because the menu's width factors into the overflow container's size calculation. It seems like there should be a less awkward way to do this, but I can't find it. Bug: 862005 Change-Id: Ib17ed810855041226786b16ca94f0a60d88dc892 Reviewed-on: https://chromium-review.googlesource.com/1145133 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Leonard Grey <lgrey@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#576905}(cherry picked from commit 86341923722bec89a92a681b582d5e465c690541) Reviewed-on: https://chromium-review.googlesource.com/1146780 Reviewed-by: Leonard Grey <lgrey@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#12} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/d3524fc6789095cfe23bee396f26e7c458033552/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
,
Jul 25
Able to reproduce the issue on chrome reported version 69.0.3486.0(Build without fix) Verified the fix on Mac 10.12.6 on Chrome version #69.0.3497.12 as per the comment#0 Attaching screencast for reference. Observed "able to see all hidden installed and active Chrome extensions in the menu" Hence, the fix is working as expected. Adding the verified label. Thanks! |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by meh...@chromium.org
, Jul 10