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

Issue 833973 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

[MacViews-Browser] Regression: Sub Folder in Bookmarks Bar Folder is not visible in Incognito Mode

Project Member Reported by meh...@chromium.org, Apr 17 2018

Issue description

Chrome Version:Canary 68.0.3398.0 
OS: macOS 10.12.6

What steps will reproduce the problem?
(1) Enable MacViews-Browser
(2) Open an Incognito Window
(3) Click on a BMB Folder, so that the Menu appears
(4) Add a sub folder into that folder

What is the expected result?
To see the Sub folder.

What happens instead?
The sub folder is very hard to see.

Screenshots are attached.

This is a regression from https://chromium-review.googlesource.com/1012966

lgrey@: Can you please take a look?

Thanks :-)
Mehmet
 
Incognito Window.png
27.6 KB View Download
Normal Window.png
30.7 KB View Download
Labels: MacViews-Browser Target-68
Owner: ellyjo...@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by gov...@chromium.org, Apr 25 2018

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.

Comment 3 by meh...@chromium.org, May 24 2018

Cc: nyerramilli@chromium.org rbasuvula@chromium.org
 Issue 846274  has been merged into this issue.
Labels: -Target-68 Target-69
Labels: MacViews-Release
Labels: -MacViews-Release
Labels: -M-68 Group-Visual_Defects
Labels: M-68
Labels: -M-68 M-69
 Issue 868825  has been merged into this issue.
Looked into this a little before I realized it was a dupe.

We're using the parent widget to set the folder color https://cs.chromium.org/chromium/src/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc but not the text color.

This works on non-Mac platforms because it seems like we make the top chrome much darker on Mac in incognito? (Not sure on Windows, but on Linux incognito just looks greyer to me)

A worse-is-better fix might be to just ifdef that line to ignore the theme on Mac, especially since issue 829891 which prompted this code isn't an issue on Mac (we use the system icons so the only choices are light and dark).
Labels: -M-69 -Target-69 M-70 Target-70
Supporting dark mode in menus will make this better, but won't make M69.
Owner: lgrey@chromium.org
mac triage: -> lgrey@
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 28

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

commit a6e5208337d8981500ff24bd25e7eb6c7926e5bc
Author: Leonard Grey <lgrey@chromium.org>
Date: Tue Aug 28 20:47:48 2018

MacViews: use dark on light icon in incognito bookmark folders

Bug:  833973 
Change-Id: I12adaf55546312ab6f348abca0671358eec5757d
Reviewed-on: https://chromium-review.googlesource.com/1194463
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586842}
[modify] https://crrev.com/a6e5208337d8981500ff24bd25e7eb6c7926e5bc/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc

Cc: krajshree@chromium.org
Labels: Needs-Feedback
Able to reproduce the issue on Mac 10.13.3 using chrome reported version #68.0.3398.0

Tested the fix on Mac 10.13.3 using latest chrome version #70.0.3537.0 as per the comment #0.
Attaching screen cast and screen shot for reference.
Observed that on creating a folder at BMB, doesn't render properly in incognito mode (an issue) but the sub folder in Bookmarks Bar Folder is visible in Incognito Mode.

lgrey@ - Could you please check the attached screen cast and screenshot and help us in confirming the fix.

Thanks...!!

Screen Shot 2018-08-30 at 16.34.17.png
37.4 KB View Download
833973.mp4
658 KB View Download
Status: Fixed (was: Assigned)
The above seems like a separate issue with bookmark button height when no button is showing (for example, do the same thing, but instead of creating a folder, bookmark a page).

I'll file a separate issue for that.
Filed as  Issue 879154 
Cc: meh...@chromium.org
 Issue 884195  has been merged into this issue.

Sign in to add a comment