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

Issue 749802 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

Put Customize Touch Bar menu item behind the flag

Project Member Reported by spqc...@chromium.org, Jul 27 2017

Issue description

The menu item is not behind a flag. This causes the menu item to appear regardless of the flag
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 27 2017

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

commit 210e2ede6cd475c57e7014b48220790b916d0caf
Author: spqchan <spqchan@chromium.org>
Date: Thu Jul 27 23:17:40 2017

[Mac] Hide the "Customize Touch Bar" menu item

The menu item needs to be removed from the main
menu when the kBrowserTouchBar flag is disabled.

Bug:  749802 
Change-Id: I71258f8f9cd9adba433444c6656a6e6a0699fe18
Reviewed-on: https://chromium-review.googlesource.com/590696
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490081}
[modify] https://crrev.com/210e2ede6cd475c57e7014b48220790b916d0caf/chrome/browser/app_controller_mac.mm

Labels: Merge-Request-60
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 27 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: kavvaru@chromium.org
Labels: Needs-Feedback
spqchan@ could you please provide us test steps and expected result to verify the issue if it requires manual verification.Also please confirm is this specific to Mac touchbar only?

Thanks,
To test this, you'll need a Macbook Pro with a TouchBar

1) Open chrome://flags and disable #mac-touchbar
2) Restart chrome and open the View menu
3) The item "Customize Touch Bar" shouldn't appear
4) Open chrome://flags and enable #mac-touchbar
5) Restart chrome and open the View menu
6) The item "Customize Touch Bar" should appear
Can you please confirm if this requires a full respin for M60 or can it wait until M61? Since M60 has launched into stable, the bar is very high for merges to M60 and only accepting critical fixes. Based on the bug description, my recommendation is to wait until M61. 
This is no longer necessary. Let's merge it to M61, thanks
Labels: -Merge-Review-60 Merge-Rejected-60
SGTM, thanks for confirming.
Labels: Merge-Request-61
Project Member

Comment 10 by sheriffbot@chromium.org, Aug 1 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

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

Comment 11 by bugdroid1@chromium.org, Aug 2 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9e8eeaea3deadf51eaf39ca4741a52d74dd6b87

commit f9e8eeaea3deadf51eaf39ca4741a52d74dd6b87
Author: spqchan <spqchan@chromium.org>
Date: Wed Aug 02 16:42:54 2017

[Mac] Hide the "Customize Touch Bar" menu item

The menu item needs to be removed from the main
menu when the kBrowserTouchBar flag is disabled.

(cherry picked from commit 210e2ede6cd475c57e7014b48220790b916d0caf)

Bug:  749802 
Change-Id: I71258f8f9cd9adba433444c6656a6e6a0699fe18
Reviewed-on: https://chromium-review.googlesource.com/590696
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490081}
Reviewed-on: https://chromium-review.googlesource.com/596421
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#239}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/f9e8eeaea3deadf51eaf39ca4741a52d74dd6b87/chrome/browser/app_controller_mac.mm

Status: Fixed (was: Assigned)
Labels: TE-Verified-61.0.3163.31
Verified the fix on Mac OSX 10.12.5 with Chrome version 61.0.3163.31, Followed steps from comment#5

Sign in to add a comment