876871 - vertical crop on focus indicator on OS X / Australis

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[David Ascher (:davida)]

Attachment: screenshot

When I focus on the Google search field on OS X, the halo is cropped at the top and bottom, breaking the glowing metaphor.  See screenshot attachment.

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20130527 Firefox/24.0

Comment 1

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

Not taking this for Australis:M7.

Comment 2

[Joshua Smith [:joshua-s]]

Attachment: Issue with downloads button

I don't know if there are bugs for these too, but the downloads button is having the same problem.

Comment 3

[Joshua Smith [:joshua-s]]

*is a bug for this.

Comment 4

[Justin Dolske [:Dolske]]

Doesn't seem as severe for me (Retina, 10.8) with a current build, vs screenshot. Did this get better?

Comment 5

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #4)
> Doesn't seem as severe for me (Retina, 10.8) with a current build, vs
> screenshot. Did this get better?

It did. We can ensure this just doesn't happen by changing our overflow settings for the overflowable toolbar. This will also mitigate bug 886939. Patch in a second.

Comment 6

[:Gijs (he/him)]

Attachment: Overflowable toolbar should only overflow in x direction,

Trivial patch.

Comment 7

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/5a056faa9b62

Comment 8

[:Gijs (he/him)]

Attachment: don't have a scrollbar instead of nothing,

Except, of course, this sets overflow-y: auto still, which doesn't really do what we want it to do. In fact, because the back button is 31px high, with a negative margin, we trigger a vertical scrollbar on OS X. Sigh.

Comment 9

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/f76156665656

Comment 10

[:Gijs (he/him)]

Backed out in bug 0e19ff24a086 because using overflow: hidden in one direction means that even if you explicitly set overflow: visible, Gecko will actually render it as overflow: auto/scroll. See bug 353599. I'm hoping this will make nightlies so that we don't get a slew of "my navbar has a scrollbar" bugs filed.

Seems like the only way to fix this permanently would be to increase the inner height of the toolbar with a min-height, or stop using overflow altogether. I'm not sure if the latter is possible. Unassigning as I'm not sure of what a permanent fix here would entail. :-(

Comment 11

[Dão Gottwald [:dao]]

Attachment: patch

How about this?

Comment 12

[Dão Gottwald [:dao]]

Attachment: patch v2

or even this, if that other patch isn't enough (I don't have OS X available to test this)

Comment 13

[:Gijs (he/him)]

Comment on attachment 809075 [details] [diff] [review]
patch v2

This wfm. I believe that per spec, we need to change the horizontal padding on the navbar to be less than 4px (that is, the back button is currently too far out of the border), but we can take care of that in another bug.

Comment 14

[:Gijs (he/him)]

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

(In reply to :Gijs Kruitbosch from comment #10)
> Backed out in bug 0e19ff24a086 because using overflow: hidden in one
> direction means that even if you explicitly set overflow: visible, Gecko
> will actually render it as overflow: auto/scroll. See bug 353599. I'm hoping
> this will make nightlies so that we don't get a slew of "my navbar has a
> scrollbar" bugs filed.

BTW, note that this sadly didn't happen. So today's nightly, at least on OS X (but possibly elsewhere) will have a vertical scrollbar in the navbar. Sorry. (we can't retrigger nightlies because of bug 846104)

Comment 15

[Dão Gottwald [:dao]]

https://hg.mozilla.org/projects/ux/rev/f74f64b2e35b

Comment 16

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/f74f64b2e35b