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

Issue 756609 link

Starred by 6 users

Chrome browser issue: Can not hide managed bookmarks in MacOs

Project Member Reported by vkhabarov@chromium.org, Aug 17 2017

Issue description

Chrome version: 60.0.3112.101 and 60.0.3112.90 (Official Build) (64-bit)
OS version: Sierra 10.12.6
Case#: 13417816

Description:
Customer reporting that 'Managed Bookmarks' folder in Chrome browser for Mac are not hidden as expected when unselecting them from the bookmark folder view. 

Steps to reproduce: 
(1) Enforce a Managed Bookmarks folder in the Admin console (Apps > G Suite > Settings for Chrome Management > Advanced Settings > User Experience) 
(2) Sign into a Chrome browser on an account the policy in enforced upon. 
(3) Attempt to uncheck the bookmarks folder from view. 

Current Behavior / Reproduction: 
Managed bookmarks are not hidden and they remain visible. 

Expected Behavior:
Managed folders in Chrome Browser should be hidden when unchecking the bookmarks folder from view.  

Was working fine in v59, works fine on other platforms

 
Cc: blumberg@chromium.org pastarmovj@chromium.org ligim...@chromium.org
Components: Enterprise
Labels: -Type-Bug Needs-Triage-M60 Needs-Bisect Type-Bug-Regression
Labels: ReleaseBlock-Stable

Comment 3 by hdodda@chromium.org, Aug 18 2017

Cc: hdodda@chromium.org bustamante@chromium.org
Labels: -Pri-2 -Needs-Bisect -M-60 hasbisect-per-revision M-62 Pri-1
Owner: lgrey@chromium.org
Status: Assigned (was: Untriaged)
Able to reproduce the issue on Mac OS 10.12.6 using chrome M62 #62.0.3188.0 and M60 #60.0.3112.101.

This is a regression issue broken in M60 and is reproduced only in Mac OS.

Using the per-revision bisect providing the bisect results,
Good build: 60.0.3088.0(Revision: 468845).
Bad build: 60.0.3089.0(Revision: 469220).

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

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

  https://chromium.googlesource.com/chromium/src/+log/1e21bd3a8b2257bf96283f1ab9a12f09e55b3acf..cdde49adc3b921524027132523cb538bfa670cd4

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

@lgrey- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

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

Thanks!

Comment 4 by ajha@chromium.org, Aug 18 2017

Cc: a...@chromium.org manoranj...@chromium.org ellyjo...@chromium.org ajha@chromium.org
Labels: -M-62 M-60
Cc'ing the reviewers, as lgrey@ is OOO until 8/21. Adjusting the milestone to M-60 based on the regression range and review of this if we plan another stable refresh.
Cc: gov...@chromium.org
Labels: M-61
Adding M61 in case they want to take a fix.

Comment 6 by gov...@chromium.org, Aug 18 2017

Cc: pbomm...@chromium.org

Comment 7 by gov...@chromium.org, Aug 21 2017

[Bulk Edit]
URGENT - PTAL.
M61 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. 

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M62. Thank you!

Note: We will only have 2 beta releases before Stable promotion. Plan is to cut M61 Stable RC on 08/31/17.

Comment 8 by lgrey@chromium.org, Aug 21 2017

hdodda@ do we have a test GSuite account I can use for repro?
lgrey@, emailed you the credentials.

Comment 10 by lgrey@chromium.org, Aug 21 2017

Cc: shrike@chromium.org

Comment 11 by lgrey@chromium.org, Aug 22 2017

Status: Started (was: Assigned)

Comment 12 by ajha@chromium.org, Aug 23 2017

Attaching the actual and expected screen-cast for clearer picture.
756609_actual.webm
742 KB View Download

Comment 13 by ajha@chromium.org, Aug 23 2017

756609_expected.webm
901 KB View Download
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 23 2017

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

commit bb6a43273c5bc1388a958c0417700c61f36deda4
Author: Leonard Grey <lgrey@chromium.org>
Date: Wed Aug 23 17:37:22 2017

[Mac] Fix toggle for showing/hiding managed bookmarks button

Bug:  756609 
Change-Id: I00271124e1c5393620da2b8dc86187924c55abe2
Reviewed-on: https://chromium-review.googlesource.com/627397
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496724}
[modify] https://crrev.com/bb6a43273c5bc1388a958c0417700c61f36deda4/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
[modify] https://crrev.com/bb6a43273c5bc1388a958c0417700c61f36deda4/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm

Cc: ranjitkan@chromium.org
Labels: TE-Verified-M62 TE-Verified-62.0.3196.0
Rechecked this issue on Chrome version 62.0.3196.0 on MAC 10.12.6. Fix is working as intended. Adding TE-Verified labels for M62.

Thanks.!

Comment 16 by lgrey@chromium.org, Aug 25 2017

Status: Fixed (was: Started)
govind@ does this need to be cherry-picked to 61?
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-60; 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-60 label, otherwise remove Merge-TBD label. Thanks.
Re #16, how safe is the change for M61 merge?




Comment 19 by lgrey@chromium.org, Aug 25 2017

It should be fine, nothing tricky

Comment 20 by lgrey@chromium.org, Aug 25 2017

Specifically, it's scoped very closely to the managed bookmark folder button, and doesn't affect non-Mac code.

Comment 21 by lgrey@chromium.org, Aug 25 2017

Labels: Merge-Request-61
Labels: -Merge-TBD -Merge-Request-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on offline chat with lgrey@ and per comment #15, #19 and #20. Please merge ASAP. Thank you.
Project Member

Comment 23 by bugdroid1@chromium.org, Aug 25 2017

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

commit b7675a5a75acf1ab39b18f430d8637bf5c24f0ab
Author: Leonard Grey <lgrey@chromium.org>
Date: Fri Aug 25 18:37:38 2017

[Mac] Fix toggle for showing/hiding managed bookmarks button

TBR=lgrey@chromium.org

(cherry picked from commit bb6a43273c5bc1388a958c0417700c61f36deda4)

Bug:  756609 
Change-Id: I00271124e1c5393620da2b8dc86187924c55abe2
Reviewed-on: https://chromium-review.googlesource.com/627397
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#496724}
Reviewed-on: https://chromium-review.googlesource.com/636303
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#893}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/b7675a5a75acf1ab39b18f430d8637bf5c24f0ab/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
[modify] https://crrev.com/b7675a5a75acf1ab39b18f430d8637bf5c24f0ab/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm

Sign in to add a comment