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

Issue 594847 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 547953



Sign in to add a comment

Location bar items no longer centered under Material Design Incognito mode

Project Member Reported by shrike@chromium.org, Mar 15 2016

Issue description

The location bar in Material Design in Incognito mode draws a 1px shadow at the bottom. To draw this shadow I stole a pixel of height from the interior of the location bar. This decreased the area within the location bar, causing icons and other content that is vertically centered to be off-center in Incognito mode. I need to find a different way to draw the 1px shadow.
 

Comment 1 by shrike@chromium.org, Apr 21 2016

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 27 2016

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

commit c1bb0685bfb2257b22548240af9c622402697780
Author: shrike <shrike@chromium.org>
Date: Wed Apr 27 17:40:38 2016

[Mac][Material Design] Rework how location bar shadow is drawn.

The location bar in Material Design has a very faint line of shadow
beneath it in Incognito mode. I originally accomplished this by drawing
it within the bounds of the location bar but by borrowing a pixel row I
changed the height of the interior of the location bar, causing items
within the bar to no longer be vertically centered. This change moves
the shadow into a separate view.

R=avi@chromium.org
BUG= 594847 

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

[modify] https://crrev.com/c1bb0685bfb2257b22548240af9c622402697780/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.h
[modify] https://crrev.com/c1bb0685bfb2257b22548240af9c622402697780/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
[modify] https://crrev.com/c1bb0685bfb2257b22548240af9c622402697780/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm

Comment 3 by shrike@chromium.org, Apr 27 2016

Status: Fixed (was: Started)

Comment 4 by shrike@chromium.org, Apr 27 2016

Status: Started (was: Fixed)
My fix can cause a crash when installing a custom theme, so have to rework it.

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 27 2016

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

commit ed1eee6bf1253dcce9eb0d8a0ef0bee86c70f3ac
Author: shrike <shrike@chromium.org>
Date: Wed Apr 27 22:12:26 2016

Revert of [Mac][Material Design] Rework how location bar shadow is drawn. (patchset #4 id:60001 of https://codereview.chromium.org/1905163002/ )

Reason for revert:
Change can cause a crash when installing a theme.

Original issue's description:
> [Mac][Material Design] Rework how location bar shadow is drawn.
>
> The location bar in Material Design has a very faint line of shadow
> beneath it in Incognito mode. I originally accomplished this by drawing
> it within the bounds of the location bar but by borrowing a pixel row I
> changed the height of the interior of the location bar, causing items
> within the bar to no longer be vertically centered. This change moves
> the shadow into a separate view.
>
> R=tapted@chromium.org
> BUG= 594847 

TBR=tapted@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 594847 

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

[modify] https://crrev.com/ed1eee6bf1253dcce9eb0d8a0ef0bee86c70f3ac/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.h
[modify] https://crrev.com/ed1eee6bf1253dcce9eb0d8a0ef0bee86c70f3ac/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
[modify] https://crrev.com/ed1eee6bf1253dcce9eb0d8a0ef0bee86c70f3ac/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm

Comment 6 by meh...@chromium.org, Apr 28 2016

Hi shrike@. I tested the change in Chromium before the revert and I noticed that the 1pt shadow below the Omnibox has no longer a fade effect. It this intended? Please see the screenshot attached (top before and below after the change). Thanks. Mehmet
Bildschirmfoto 2016-04-27 um 23.49.34.png
30.3 KB View Download

Comment 7 by shrike@chromium.org, Apr 28 2016

Hi mehmet@ - can you redescribe what you're seeing? I'm not sure what you mean by "fade effect".

Comment 8 by meh...@chromium.org, Apr 28 2016

Of course, I attached a new screenshot showing it with red arrows. I mean the the 1pt shadow below the Omnibox.

Top: Before the change. The 1pt border is not so visible and has a kind of fade effect.

Bottom: After the change. The 1pt border is very dark and it looks like the Omnibox has 2pt bottom border.
054401f7-ebc0-4be4-a7d3-69ca10f11785.png
32.1 KB View Download

Comment 9 by shrike@chromium.org, Apr 28 2016

Thank you. I will double-check to make sure it's doing the right thing.
Great, thanks :-)
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 29 2016

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

commit 56996c30e8a829fb4f7e4733845717e23f9f1b14
Author: shrike <shrike@chromium.org>
Date: Fri Apr 29 21:21:54 2016

[Mac][Material Design] Rework how location bar shadow is drawn.

The location bar in Material Design has a very faint line of shadow
beneath it in Incognito mode. I originally accomplished this by drawing
it within the bounds of the location bar but by borrowing a pixel row I
changed the height of the interior of the location bar, causing items
within the bar to no longer be vertically centered. This change moves
the shadow into a separate view.

R=tapted@chromium.org
BUG= 594847 

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

[modify] https://crrev.com/56996c30e8a829fb4f7e4733845717e23f9f1b14/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.h
[modify] https://crrev.com/56996c30e8a829fb4f7e4733845717e23f9f1b14/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
[modify] https://crrev.com/56996c30e8a829fb4f7e4733845717e23f9f1b14/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm

Labels: -M-51 M-52
Status: Fixed (was: Started)
I meant to look into the shadow issue before landing this cl but that did not happen. I see on my non-Retina machine that the shadow is dark. I'm preparing another cl that tweaks the shadow drawing color so I will fix this issue there.

(that is  Issue 605140 )
Thank you. Starred the issue. 
It looks like the shadow on non-Retina is lightened up in the new cl (it's in the process of landing).

Sign in to add a comment