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

Issue 734476 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unnecessary dark black border is seen beneath bookmark bar if ’Show bookmark bar’ option is disable

Reported by abom...@etouch.net, Jun 19 2017

Issue description

Chrome Version:61.0.3134.0 (Official Build)  d6653466e313dc83a33ec049a6eef95a31dda435-refs/heads/master@{#480301}
OS: MAC (10.12.3,10.11.6)

Pre-condition:Show bookmark bar’ option should be disable.

What steps will reproduce the problem?
1. Launch chrome ,Open NTP and observe beneath bookmark bar

Actual: Unnecessary dark black border is seen beneath bookmark bar
Expected: Unnecessary dark black border should not be seen beneath bookmark bar

This is regression issue, broken in ‘M 61’ and below is manual bisect:
Good build:61.0.3132.0
Bad build:61.0.3134.0

Note:Issue is not seen on Linux and Windows OS.
 
Actual_border.png
18.2 KB View Download
Expected_border.png
14.8 KB View Download
Cc: jmukthavaram@chromium.org
Labels: hasbisect-per-revision ReleaseBlock-Stable
Owner: est...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on MAc 10.12.5 using chrome #61.0.3134.0 as per the above steps:
Manual Bisect info:
------------------
Good build:61.0.3132.0-Revision-479900
Bad build:61.0.3134.0-Revision-480301

Per revision bisect info:
------------------------
You are probably looking for a change made after 480274 (known good), but no later than 480275 (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/917d881740c08a34d7a83ec324fe3c4854f19de0..eba855f22deff3a00b76384582808925841f4be5

Possible suspect:
---------------
https://chromium.googlesource.com/chromium/src/+/eba855f22deff3a00b76384582808925841f4be5

Estade@Could you please take a look & reassign to the right owner if it is not related to your change.

Adding 'Release block stable' label as it is regressed recently.Please remove if it is not the case.
Thanks..!!

Comment 2 by est...@chromium.org, Jun 20 2017

Cc: est...@chromium.org
Owner: tapted@chromium.org
So it appears that unlike Views, Cococa draws the bottom stroke opaquely (as opposed to on top of the bg color, honoring alpha). The code is here[1]. +tapted, do you know how to fix this or who else might have the cycles to help? Seems like it should be roughly a one liner.

[1] https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_toolbar_view.mm?rcl=5ea087fcae356e2f872d63d4aed4aecf20169135&l=82

Comment 3 by tapted@chromium.org, Jun 21 2017

It should be compositing on the background..

My guess is that the line

    strokeColor = [strokeColor colorWithAlphaComponent:0.5];

is replacing the 0x20 alpha with 0x80 alpha rather than multiplying it to be 0x10 alpha.

The fix is probably just to delete that line... I'll take a peep.

Comment 4 by tapted@chromium.org, Jun 21 2017

Status: Started (was: Assigned)
Deleting seems to work. Attached is left-to-right: dev, canary, fixed@HEAD

With the fix, the color is a *tiny* bit lighter. Certainly in incognito. But I think that was the intent of r480275

https://codereview.chromium.org/2946183002
Screen Shot 2017-06-21 at 11.10.36 am.png
19.6 KB View Download
Screen Shot 2017-06-21 at 11.12.42 am.png
20.1 KB View Download

Comment 5 by est...@chromium.org, Jun 21 2017

that wasn't technically the intent of that change, but I think this brings Cocoa into better alignment with Views.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 22 2017

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

commit 36d91d332d9c8d867cd564f958e7406b7ef893c9
Author: tapted <tapted@chromium.org>
Date: Thu Jun 22 00:34:19 2017

Mac: Don't replace the alpha on the detached toolbar separator color.

The Cocoa browser used to clobber the theme's alpha with 50%. But since
r480275, theme_service.cc makes appropriate adjustments to the color
instead.

BUG= 734476 

Review-Url: https://codereview.chromium.org/2946183002
Cr-Commit-Position: refs/heads/master@{#481375}

[modify] https://crrev.com/36d91d332d9c8d867cd564f958e7406b7ef893c9/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_toolbar_view.mm

Comment 7 by tapted@chromium.org, Jun 22 2017

Status: Fixed (was: Started)

Comment 8 by abom...@etouch.net, Jun 27 2017

Labels: TE-Verified-M61 TE-Verified-61.0.3141.0 TE-Verified-61.0.3142.0
Above issue seems to be fixed to Latest Dev Version:61.0.3141.0 (Official Build) 180095eb1bca7df1cdcb02547340499c2ee3af6e-refs/heads/master@{#482153} and Latest Canary Version:61.0.3142.0 (Official Build) using Mac Pro (10.12.3 & 10.11.6)

Sign in to add a comment