873662 - Australis causes back button to have an incorrect gradient/clip-path

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Josiah Bruner [:jsbruner]]

Attachment: Back button issue.

On the current UX builds, OS X has a slight white smudge towards the bottom right of the back button. See attached image for reference.

Does not occur on the normal Nightly. Linux and Windows are unaffected.

Comment 1

[Josiah Bruner [:jsbruner]]

Attachment: Button size issue with lw-themes

Alright, so the gradient color issue does not occur when lw-themes are applied. However, applying lw-themes causes the toolbarbuttons to gain quite a bit of height.

Adding a screenshot that demonstrates the issue. (This new issue can not be reproduced on Windows, which makes me think that they both are the related)

Comment 2

[Brandon Cheng]

This is caused by a mis-aligned clip-path. I encountered it in bug 873626. I should be able to fix this after a bit of chrome inspecting.

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: WIP: Adjust clipPath for the back button

Since I needed to learn SVG, this appeared to me as a Good First Bug(tm)!

While working on this I discovered this epic tool - that you prolly already know - http://svg-edit.googlecode.com/svn/branches/2.6/editor/svg-editor.html

Comment 4

[Brandon Cheng]

(In reply to Mike de Boer [:mikedeboer] from comment #3)
> Created attachment 753465 [details] [diff] [review]
> Adjust clipPath for the back button
> 
> Since I needed to learn SVG, this appeared to me as a Good First Bug(tm)!
> 
> While working on this I discovered this epic tool - that you prolly already
> know - http://svg-edit.googlecode.com/svn/branches/2.6/editor/svg-editor.html

This bug currently depends on bug 873626. Once a patch for that is created for OS X, the clip-path will need to be adjusted again. I should have marked that down here when I was assigned to this bug.

Additionally, Dão mentioned in that same bug that it would be better to move the clip-path to the #urlbar instead of its container (and I agree). So we'll have another change once I get to that.

Comment 5

[Mike de Boer [:mikedeboer]]

(In reply to Brandon Cheng from comment #4)
> This bug currently depends on bug 873626. Once a patch for that is created
> for OS X, the clip-path will need to be adjusted again. I should have marked
> that down here when I was assigned to this bug.
> 
> Additionally, Dão mentioned in that same bug that it would be better to move
> the clip-path to the #urlbar instead of its container (and I agree). So
> we'll have another change once I get to that.

In that case I'll assign this bug back to you, because you're on top of it ;) Consider the patch I filed WIP.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Hey Brandon,

Just a heads up that we're hoping to have this bug closed by next Wednesday. Do you think you'll be able to prepare a patch and get it reviewed by then?

-Mike

Comment 7

[Brandon Cheng]

(In reply to Mike Conley (:mconley) from comment #6)
> Hey Brandon,
> 
> Just a heads up that we're hoping to have this bug closed by next Wednesday.
> Do you think you'll be able to prepare a patch and get it reviewed by then?
> 
> -Mike

Sorry Mike. I should have a patch ready at the end of tomorrow (Friday). :)

Comment 8

[Brandon Cheng]

Attachment: Patch v1

This bug was originally caused by a growth of the urlbar-container by +10px (5px on top and bottom) from stable to ux. I'm not sure if this was intentional. If anyone has the bug id this change was made in, I'd love to take a look.

This patch updates references 5px down.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 757055 [details] [diff] [review]
Patch v1

Review of attachment 757055 [details] [diff] [review]:
-----------------------------------------------------------------

Did you notice the size change by using DOM Inspector? I looked through themes/osx/browser.css and didn't see any vertical margins or padding for the urlbar-container.

Comment 10

[Brandon Cheng]

Yes. I had a userstyle that just painted the url-container black. The black extended past the urlbar for UX, and not for stable.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Landed in UX as https://hg.mozilla.org/projects/ux/rev/33315aa543f5

Comment 12

[Justin Dolske [:Dolske]]

Attachment: Hmm, not quite

Patch v1 made this a lot better, but it's still not quite right. (Zoomed screenshot attached). Notice how the blue glow at the top directly touches the dark border, whereas at the bottom it's a couple pixels away (or touching a light border hilight?). I assume the top and bottom should contact in the same way.

Also, is there a separate bug for the way the blue glow doesn't extend to the left far enough? (This is a problem independent from the clippath.)

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Justin Dolske [:Dolske] from comment #12)
> Also, is there a separate bug for the way the blue glow doesn't extend to
> the left far enough? (This is a problem independent from the clippath.)

See both bug 703063 and bug 714170.

Comment 14

[Mike de Boer [:mikedeboer]]

(In reply to Jared Wein [:jaws] from comment #13)
> (In reply to Justin Dolske [:Dolske] from comment #12)
> > Also, is there a separate bug for the way the blue glow doesn't extend to
> > the left far enough? (This is a problem independent from the clippath.)
> 
> See both bug 703063 and bug 714170.

This will be fixed in bug 755598. This doesn't include bug 714170

Comment 16

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/33315aa543f5