Fix spacing in location bar to be less uneven

VERIFIED FIXED in Firefox 42

Status

()

Firefox
Theme
P4
normal
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: Gijs, Assigned: dao)

Tracking

Trunk
Firefox 44
Points:
---

Firefox Tracking Flags

(firefox42 verified, firefox43 verified, firefox44 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
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?
Flags: needinfo?(philipp)
(Reporter)

Comment 1

3 years ago
Created attachment 8659426 [details]
Screenshot
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?
(Assignee)

Updated

2 years ago
Keywords: uiwanted
(Assignee)

Comment 3

2 years ago
Lack of response from UX suggests this is low priority...
Priority: -- → P4
(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
(Assignee)

Comment 5

2 years ago
(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.
Flags: needinfo?(philipp)
Keywords: uiwanted
(Assignee)

Comment 6

2 years ago
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.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8670115 - Flags: review?(gijskruitbosch+bugs)
(Reporter)

Comment 7

2 years ago
(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.
(Reporter)

Comment 8

2 years ago
Comment on attachment 8670115 [details] [diff] [review]
patch

Tested on OSX and Linux, I'm happy to take your word for it on Windows.
Attachment #8670115 - Flags: review?(gijskruitbosch+bugs) → review+
(Reporter)

Comment 9

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

bug 1212355
(Assignee)

Comment 11

2 years ago
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]:
Attachment #8670115 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4e33479760b9
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Comment on attachment 8670115 [details] [diff] [review]
patch

Approved for uplift; minor css/ux tweak.
Attachment #8670115 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 15

2 years ago
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]:
Attachment #8670115 - Flags: approval-mozilla-beta?
Comment on attachment 8670115 [details] [diff] [review]
patch

Should be low risk, taking it. Should be in 42 beta 6.
Attachment #8670115 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 17

2 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/53654f87de0b
status-firefox42: --- → fixed

Comment 18

2 years ago
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]
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!
Status: RESOLVED → VERIFIED
QA Whiteboard: [testday-20151016]
status-firefox42: fixed → verified
status-firefox43: fixed → verified
status-firefox44: fixed → verified
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]
You need to log in before you can comment on or make changes to this bug.