1205100 - Text in buttons on about:support causes overflow

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[arni2033]

Attachment: screenshot - Text in buttons on about-support causes overflow on HiDPI.png

Nightly 43, ID 20150915030232
It's very noticeable on HiDPI but also is reproducible with scale=100%. To reproduce this on HiDPI and English locale you may need to reduce the window's width.

Comment 1

[:Gijs (he/him)]

Attachment: Patch

Comment 2

[Pulsebot]

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

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/fb266acf3160 for crashes:

https://treeherder.mozilla.org/logviewer.html#?job_id=4707790&repo=fx-team

Comment 4

[:Gijs (he/him)]

(In reply to Wes Kocher (:KWierso) from comment #3)
> Backed out in https://hg.mozilla.org/integration/fx-team/rev/fb266acf3160
> for crashes:
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=4707790&repo=fx-team

WTF. Fancy that for a one-line CSS change. :-\

Comment 5

[arni2033]

Offtopic
Actually, if (on Win7) I set HWA -> on, your change causes buttons to have height=31px, while with HWA=off their height=30px (as expected). Is that bug worth reporting or will it be marked Invalid?

Comment 6

[:Gijs (he/him)]

Filed bug 1205620 and bug 1205622. Will re-land with a fix to not touch about:addons layout.

Comment 7

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #6)
> Filed bug 1205620 and bug 1205622. Will re-land with a fix to not touch
> about:addons layout.

I've changed my mind - this actually breaks a lot of things on OS X, like e.g. about:preferences menulists, which have min-height: 20px !important, which overrides this and therefore they now end up being 10px smaller.

It seems like a much simpler fix will be to fix this specifically in aboutSupport. I don't think we really end up with many in-content buttons with text that long anyway. :-\

Comment 8

[:Gijs (he/him)]

Attachment: fix text wrapping in buttons on about:support,

Comment 9

[:Gijs (he/him)]

(In reply to arni2033 from comment #5)
> Offtopic
> Actually, if (on Win7) I set HWA -> on, your change causes buttons to have
> height=31px, while with HWA=off their height=30px (as expected). Is that bug
> worth reporting or will it be marked Invalid?

31px according to the DOM or 31px visually? It sounds worth reporting to me, but as I won't be landing the original patch, I expect it'd be useful to have a more minimal testcase for that problem.

Comment 10

[:Gijs (he/him)]

(to be clear, that'd be a core bug, and a surprising one at that... though actually, now that I think about it, it might have something to do with the fonts in use and how different methods of rendering those would affect their size, which will cause the "natural" height of the text to be different, which might not in and of itself be a bug)

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8662341 [details] [diff] [review]
fix text wrapping in buttons on about:support,

(In reply to :Gijs Kruitbosch from comment #7)
> I've changed my mind - this actually breaks a lot of things on OS X, like
> e.g. about:preferences menulists, which have min-height: 20px !important,
> which overrides this and therefore they now end up being 10px smaller.

We can fix that by removing the stray !important, correct? Likewise, other regressions just need to dealt with piece by piece. Not all of that needs to be taken care of by you let alone in this bug; I'm fine with just fixing the most obvious stuff and then landing it to seeing what else breaks.

The hardcoded height in common.css is wrong and we should fix that sooner rather than later, otherwise the crap keeps just piling up as we use common.css in more places.

Comment 12

[Pulsebot]

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

Comment 13

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #11)
> Comment on attachment 8662341 [details] [diff] [review]
> fix text wrapping in buttons on about:support,
> 
> (In reply to :Gijs Kruitbosch from comment #7)
> > I've changed my mind - this actually breaks a lot of things on OS X, like
> > e.g. about:preferences menulists, which have min-height: 20px !important,
> > which overrides this and therefore they now end up being 10px smaller.
> 
> We can fix that by removing the stray !important, correct? Likewise, other
> regressions just need to dealt with piece by piece. Not all of that needs to
> be taken care of by you let alone in this bug; I'm fine with just fixing the
> most obvious stuff and then landing it to seeing what else breaks.
> 
> The hardcoded height in common.css is wrong and we should fix that sooner
> rather than later, otherwise the crap keeps just piling up as we use
> common.css in more places.

Alright! I removed the !important (there since hg@1... but I couldn't find anything that broke in browser/, at least), and s/height/min-height in the relevant add-ons stylesheet so that that continues working (see bug 1205620 for fixing the button to actually be invisible, as it's unused, and bug 1205622 for making the test not so ... unreliable).

Let's see how this goes...

Comment 14

[arni2033]

(In reply to :Gijs Kruitbosch from comment #9)
> 31px according to the DOM or 31px visually?
According to inspector in devtools (Box Model). And also, I see the buttons twitching when I remove rule "height:30px" (or replace it with rule "min-width:30px"). Filed bug 1205724 with reduced testcase.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 16

[Mohammad Maruf Islam]

I have reproduced this bug with Firefox Nightly 43.0a1 (Build ID: 20150915030232) on 
windows 8.1 64-bit with the instructions from comment 0 .

Verified as fixed with Firefox Nightly 44.0a1(Build ID:20151001030236)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0