1203661 - Fix spacing in location bar to be less uneven

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P4)

Description

[:Gijs (he/him)]

Philipp, the location bar padding on OS X around the dropmarker and stop/go/reload looks uneven after the changes from bug 1185960 and bug 1202287. Can you indicate how you want us to fix this?

Comment 1

[:Gijs (he/him)]

Attachment: Screenshot

Comment 2

[Justin Dolske [:Dolske]]

In case it's unclear: it's the differing space between the separator and drop/reload icons.

It's almost like an optical illusion... It kind makes sense if you look at the separator as being between a reload button and the rest of the bar (which happens to include a droparrow at the end). But if you look at it as a separator between two icons at the end of the url bar, it's weird.

Making the spacing equal fixes the latter, but might make the former seem weird? Another idea would be to give the reload button area a different background color, which more make it look a bit more like a separate-but-attached button.

I guess Steven's mocks at http://people.mozilla.org/~shorlander/mockups/Windows-10/Windows-10.html show equal spacing on both sides of the separator, and it looks ok to me, so that might be the simplest fix?

Comment 3

[Dão Gottwald [:dao]]

Lack of response from UX suggests this is low priority...

Comment 4

[Stephen Horlander [:shorlander]]

(In reply to Justin Dolske [:Dolske] from comment #2)
> I guess Steven's mocks at
> http://people.mozilla.org/~shorlander/mockups/Windows-10/Windows-10.html
> show equal spacing on both sides of the separator, and it looks ok to me, so
> that might be the simplest fix?

Yeah I think we should just increase the spacing between whatever the last item is and the separator. 8px looks pretty good and is technically the same amount of side padding for the reload button.

http://cl.ly/image/3I2A260h0S3s

Comment 5

[Dão Gottwald [:dao]]

(In reply to Stephen Horlander [:shorlander] from comment #4)
> 8px looks pretty good and is technically the same
> amount of side padding for the reload button.

The reload button uses 9px, but we don't have to match that exactly and can use whatever works best visually.

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch

5px margin + 3px padding on the dropmarker = 8px. On Linux the padding is part of -moz-appearance and at least on Ubuntu it looks like that's about 3px as well.

Comment 7

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #6)
> Created attachment 8670115 [details] [diff] [review]
> patch
> 
> 5px margin + 3px padding on the dropmarker = 8px. On Linux the padding is
> part of -moz-appearance and at least on Ubuntu it looks like that's about
> 3px as well.

We must be using different themes, because on my Ubuntu machine (which is running the default Adwaita, so says the Tweak Tool - but using gnome 3 instead of the unity launcher, so maybe that's the difference?) I see 0px padding, so this isn't enough.

IMO we should just make the Linux implementation not use moz-appearance here, it makes very little sense because it isn't a toolbarbutton in any traditional sense of the word. We also use the native image instead of the SVG we use on Windows & OS X - I don't really see the point of this, and it means color-wise it looks out of place compared to the other buttons anyway (it's black, but all our other icons/separators inside the textboxes are grey-ish) - and we get fun bugs like bug 121235. Both for maintenance and for consistency's sake it'd be better if we just did the same thing there as we did on Windows. Could probably pull some of the styling out to a new shared stylesheet in that case, too.

I don't really mind whether we do this here or in a new bug, but there we are.

Comment 8

[:Gijs (he/him)]

Comment on attachment 8670115 [details] [diff] [review]
patch

Tested on OSX and Linux, I'm happy to take your word for it on Windows.

Comment 9

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #7)
> fun bugs like bug 121235

bug 1212355

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/4e33479760b9

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8670115 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1185960
[User impact if declined]: see comment 2
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: trivially adjusting a single CSS property, no risk
[String/UUID change made/needed]:

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4e33479760b9

Comment 13

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8670115 [details] [diff] [review]
patch

Approved for uplift; minor css/ux tweak.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ff125ed4c174

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 8670115 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1185960
[User impact if declined]: see comment 2
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: trivially adjusting a single CSS property, no risk
[String/UUID change made/needed]:

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8670115 [details] [diff] [review]
patch

Should be low risk, taking it. Should be in 42 beta 6.

Comment 17

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-beta/rev/53654f87de0b

Comment 18

[Md. Rahimul Islam]

I have successfully reproduce this bug on firefox nightly 43.0a1 (2015-09-10)
with windows 10 (32 bit)
Mozilla/5.0 (Windows NT 10.0; rv:43.0) Gecko/20100101 Firefox/43.0

I found this fix on latest beta 42.0b7
Mozilla/5.0 (Windows NT 10.0; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID : 20151015151621
I also found this fix on latest aurora 43.0a2 (2015-10-15)
Mozilla/5.0 (Windows NT 10.0; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID :20151015004024
I found this fix on latest nightly 44.0a1 (2015-10-15)
Mozilla/5.0 (Windows NT 10.0; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID : 20151015030233

[testday-20151016]

Comment 19

[Nazir Ahmed Sabbir [:NaSb]]

Reproduced this bug on Nightly 43.0a1 (2015-09-10)(Build ID: 20150910030225) in Linux, 64 bit

This Bug is now verified as fixed on Latest Firefox Beta 42.0b7 (Build ID: 20151015151621) and

Latest Firefox Developer Edition 43.0a2 (2015-10-15) (Build ID: 20151015004024) and

Latest Firefox Nightly 44.0a1 (2015-10-15) (Build ID: 20151015030233)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0

As it is also verified on Windows (Comment 18), Marking it as verified!

Comment 20

[Bolaram Paul [:ancon]]

This bug's fix is verified in latest Firefox beta 42.0b7

Build Id:        20151015151621 
User Agent:      Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0 

[testday-20151016]