963365 - [Australis] Menu panel layout is broken

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Théo Chevalier]

Attachment: Australis menu regression, Win7 vs. Ubuntu

Bug 944947 hadn't fixed the Linux layout bug, and introduced padding regression on widgets.

Could you go back to the previous right and left paddings for Widgets, or is it already the best we can have? Because of that, we have more widgets going to 3 lines, smaller paddings could help avoiding that.
And some labels like "Mar-que-pages" are *really* ugly and hard to read, it could totally fit on one line, I think. And this particular widget is still overflowing (as you can notice on the Linux screenshot).

Last thing, some labels are not behaving the same way on Windows and Linux. Look at "Nouvelle fenêtre privée" (New private tab) for instance, 3 lines on Linux, 2 lines on Windows. It becomes tricky to adjust localization with an inconsistent behavior.

Comment 1

[:Gijs (he/him)]

(In reply to Théo Chevalier [:tchevalier] from comment #0)
> Created attachment 8364789 [details]
> Australis menu regression, Win7 vs. Ubuntu
> 
> Bug 944947 hadn't fixed the Linux layout bug, and introduced padding
> regression on widgets.
> 
> Could you go back to the previous right and left paddings for Widgets, or is
> it already the best we can have? Because of that, we have more widgets going
> to 3 lines, smaller paddings could help avoiding that.

There were no changes to the padding, as far as I know. Do you use anything other than 100% font size settings on the OS preference level on Windows?

> And some labels like "Mar-que-pages" are *really* ugly and hard to read, it
> could totally fit on one line, I think. And this particular widget is still
> overflowing (as you can notice on the Linux screenshot).

I'm not sure what bit you're talking about at this point. The fact that the hover style doesn't contain all the text, or the fact that there are 2 instead of 3 buttons on one line. As it is, this bug lists a multitude of issues; they would benefit from having their own bug...

> Last thing, some labels are not behaving the same way on Windows and Linux.
> Look at "Nouvelle fenêtre privée" (New private tab) for instance, 3 lines on
> Linux, 2 lines on Windows. It becomes tricky to adjust localization with an
> inconsistent behavior.

This is impossible to fix. The font sizes as well as panel and button widths are all in em; the actual text size depends on your OS font settings. It's never going to be pixel-perfect cross-platform, unless we hardcode all the things, in which case everyone will be sad about how we're not following their font preference settings.

Comment 2

[Théo Chevalier]

(In reply to :Gijs Kruitbosch from comment #1)
> (In reply to Théo Chevalier [:tchevalier] from comment #0)
> > Created attachment 8364789 [details]
> > Australis menu regression, Win7 vs. Ubuntu
> > 
> > Bug 944947 hadn't fixed the Linux layout bug, and introduced padding
> > regression on widgets.
> > 
> > Could you go back to the previous right and left paddings for Widgets, or is
> > it already the best we can have? Because of that, we have more widgets going
> > to 3 lines, smaller paddings could help avoiding that.
> 
> There were no changes to the padding, as far as I know. Do you use anything
> other than 100% font size settings on the OS preference level on Windows?

On Windows no, this is pretty much a Win 7 fresh install. However on Linux I reduced Ubuntu default font size.

> 
> > And some labels like "Mar-que-pages" are *really* ugly and hard to read, it
> > could totally fit on one line, I think. And this particular widget is still
> > overflowing (as you can notice on the Linux screenshot).
> 
> I'm not sure what bit you're talking about at this point. The fact that the
> hover style doesn't contain all the text, or the fact that there are 2
> instead of 3 buttons on one line. As it is, this bug lists a multitude of
> issues; they would benefit from having their own bug...

Sorry, I was talking about the hover style, yes. But as a matter of fact, it has been fixed yesterday :)


About the "Mar-que-pages", this is a bug in the French hyphens rules, the longest part (Marque-) should be in the first line. We found an updated version of the hyphens rules we use in gecko http://www.dicollecte.org/download/fr/hyph-fr-v3.0.zip I'm doing a build to see if that solves the issue. I'll open a new bug for that.

> 
> > Last thing, some labels are not behaving the same way on Windows and Linux.
> > Look at "Nouvelle fenêtre privée" (New private tab) for instance, 3 lines on
> > Linux, 2 lines on Windows. It becomes tricky to adjust localization with an
> > inconsistent behavior.
> 
> This is impossible to fix. The font sizes as well as panel and button widths
> are all in em; the actual text size depends on your OS font settings. It's
> never going to be pixel-perfect cross-platform, unless we hardcode all the
> things, in which case everyone will be sad about how we're not following
> their font preference settings.

ok, fair enough :) Thanks for the clarification.


In a nutshell, I think the only real - obvious - issue here is the third column on Linux (only 2 widgets in a row)

Comment 3

[:Gijs (he/him)]

(In reply to Théo Chevalier [:tchevalier] from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > (In reply to Théo Chevalier [:tchevalier] from comment #0)
> > > Created attachment 8364789 [details]
> > > Australis menu regression, Win7 vs. Ubuntu
> > > 
> > > Bug 944947 hadn't fixed the Linux layout bug, and introduced padding
> > > regression on widgets.
> > > 
> > > Could you go back to the previous right and left paddings for Widgets, or is
> > > it already the best we can have? Because of that, we have more widgets going
> > > to 3 lines, smaller paddings could help avoiding that.
> > 
> > There were no changes to the padding, as far as I know. Do you use anything
> > other than 100% font size settings on the OS preference level on Windows?
> 
> On Windows no, this is pretty much a Win 7 fresh install. However on Linux I
> reduced Ubuntu default font size.

Did Windows regress as to what it was before? It'd be useful to see a screenshot of the same menu in a previous Nightly.

> > > And some labels like "Mar-que-pages" are *really* ugly and hard to read, it
> > > could totally fit on one line, I think. And this particular widget is still
> > > overflowing (as you can notice on the Linux screenshot).
> > 
> > I'm not sure what bit you're talking about at this point. The fact that the
> > hover style doesn't contain all the text, or the fact that there are 2
> > instead of 3 buttons on one line. As it is, this bug lists a multitude of
> > issues; they would benefit from having their own bug...
> 
> Sorry, I was talking about the hover style, yes. But as a matter of fact, it
> has been fixed yesterday :)

Oh really? Can you elaborate? I'm not aware of us fixing the hover style yesterday, but maybe I've forgotten something... it's crunch time and it's hard to keep track of everything. :-)

> About the "Mar-que-pages", this is a bug in the French hyphens rules, the
> longest part (Marque-) should be in the first line. We found an updated
> version of the hyphens rules we use in gecko
> http://www.dicollecte.org/download/fr/hyph-fr-v3.0.zip I'm doing a build to
> see if that solves the issue. I'll open a new bug for that.

Oooh! Yes, I did think that, but then, French is only my third/fourth language depending on how I count, so I didn't want to assume I understood that correctly... Did you check the right hyphenation dictionary is definitely used? (I mean, I wrote the patch and checked with French, so I *think* it should work, but...)

> > > Last thing, some labels are not behaving the same way on Windows and Linux.
> > > Look at "Nouvelle fenêtre privée" (New private tab) for instance, 3 lines on
> > > Linux, 2 lines on Windows. It becomes tricky to adjust localization with an
> > > inconsistent behavior.
> > 
> > This is impossible to fix. The font sizes as well as panel and button widths
> > are all in em; the actual text size depends on your OS font settings. It's
> > never going to be pixel-perfect cross-platform, unless we hardcode all the
> > things, in which case everyone will be sad about how we're not following
> > their font preference settings.
> 
> ok, fair enough :) Thanks for the clarification.
> 
> 
> In a nutshell, I think the only real - obvious - issue here is the third
> column on Linux (only 2 widgets in a row)

Yes. If you're familiar with DOM Inspector, it would be helpful if you could tell me the widths and paddings (as reported by the box model view) of the PanelUI-contents, the toolbarbuttons, and the (anonymous) toolbarbutton-multiline-text elements inside the toolbarbuttons (the first three buttons should do) on your Linux machine.

Comment 4

[:Gijs (he/him)]

Ugh, I meant to needinfo and apparently forgot. I'd really appreciate some info here per comment #3. :-)

Comment 5

[Théo Chevalier]

Oh, so sorry, I get busy then I forgot to answer you... :\

(In reply to :Gijs Kruitbosch from comment #3)
> Did Windows regress as to what it was before? It'd be useful to see a
> screenshot of the same menu in a previous Nightly.

I haven't been able to find a localized nightly before 01-27 on the ftp. Aren't we supposed to keep all those builds?


> 
> Oh really? Can you elaborate? I'm not aware of us fixing the hover style
> yesterday, but maybe I've forgotten something... it's crunch time and it's
> hard to keep track of everything. :-)
Well, the root cause was, IMHO, the "Show all bookmark" button being right under the "Bookmark this page" button, even when dragged to the menu. So, two-in-one buttons was the issue.

> Oooh! Yes, I did think that, but then, French is only my third/fourth
> language depending on how I count, so I didn't want to assume I understood
> that correctly... Did you check the right hyphenation dictionary is
> definitely used? (I mean, I wrote the patch and checked with French, so I
> *think* it should work, but...)

I made a build with the updated hyphenation dictionary, but unfortunately it didn't fix anything for "mar-que-pages".
But yeah the new dictionary has a lot of differences, I filed Bug 966818 for that.


> Yes. If you're familiar with DOM Inspector, it would be helpful if you could
> tell me the widths and paddings (as reported by the box model view) of the
> PanelUI-contents, the toolbarbuttons, and the (anonymous)
> toolbarbutton-multiline-text elements inside the toolbarbuttons (the first
> three buttons should do) on your Linux machine.

Hm, sadly I'm not very familiar with DOM Inspector, I haven't been able to find out what file I should inspect.
chrome://browser/content/customizableui/panelUI.xml ?

Comment 6

[Mark Hammond [:markh] [:mhammond]]

I'm not 100% sure this is the correct bug, but on a nightly built today (and for the last couple of weeks), my Windows 7 machine looks like the Linux screen-shot - only 2 columns instead of 3 (but no scroll bar).  Windows 7 has custom DPI setting of 110%.

DOMi tells me:

PanelUI-contents: padding-top/-bottom: 5.88333px, -left and -right: 0px. width: 263, height: 458

First toolbarbutton (new-window-button): padding-top/-bottom: 2px, padding-right/-left: 6px. width: 88, height: 68
-> toolbarbutton-multiline-text: all padding: 0px, width 69, height 20

Second toolbarbutton (provatebrowsing-button): same padding as above, width: 88, height: 82
-> toolbarbutton-multiline-text: all padding: 0px, width 74, height 34

Third button (save-page-button) - same as new-window-button
-> toolbarbutton-multiline-text: all padding: 0px, width 54, height 20

Comment 7

[:Gijs (he/him)]

(In reply to Théo Chevalier [:tchevalier] from comment #5)
> > Yes. If you're familiar with DOM Inspector, it would be helpful if you could
> > tell me the widths and paddings (as reported by the box model view) of the
> > PanelUI-contents, the toolbarbuttons, and the (anonymous)
> > toolbarbutton-multiline-text elements inside the toolbarbuttons (the first
> > three buttons should do) on your Linux machine.
> 
> Hm, sadly I'm not very familiar with DOM Inspector, I haven't been able to
> find out what file I should inspect.
> chrome://browser/content/customizableui/panelUI.xml ?

In the toplevel menu, file > inspect chrome document, then select your main browser window (usually the first in the list).

Then open and close the panel menu.

Then hit ctrl+f, switch to searching by 'Attr', and then type 'id' for the name, and 'PanelUI-contents' for the value. Expand that subtree further until you can see the list of all the toolbar buttons.

On the right-hand pane, there's an icon with a dropmarker arrow next to "Object - DOM Node" - click it and switch to 'Box Model' and report some of the dimensions here, please.

Mark already did this, but I'd be curious if the issue on Linux is the same, especially since Mark said he'd been seeing this for a few weeks now, and for you it was a recent development...

Comment 8

[Théo Chevalier]

Thanks a lot for the guidance :)
Copy/pasting Mark post, updating it with my data:

PanelUI-contents: padding-top/-bottom: 5.88333px, -left and -right: 0px. width: 263, height: 952

First toolbarbutton (new-window-button): padding-top/-bottom: 2px, padding-right/-left: 6px. width: 88, height: 76, margin-top: 6px
-> toolbarbutton-multiline-text: all padding: 0px, width 74, height 28, margin-top:2px

Second toolbarbutton (provatebrowsing-button): same padding as above, width: 88, height: 76
-> toolbarbutton-multiline-text: all padding: 0px, width 74, height 28, margin-top:2px

Third button (save-page-button) - same as new-window-button
-> toolbarbutton-multiline-text: all padding: 0px, width 74, height 28, margin-top:2px

Comment 9

[:Gijs (he/him)]

Huh, interesting. What font-size is used for the toolbarbuttons? (under the "Computed Style" section in the right-hand pane, from the same dropdown where you picked "Box Model")

Comment 10

[Théo Chevalier]

(In reply to :Gijs Kruitbosch from comment #9)
> Huh, interesting. What font-size is used for the toolbarbuttons? (under the
> "Computed Style" section in the right-hand pane, from the same dropdown
> where you picked "Box Model")

font-size: 11.7667px;
font-family: "Ubuntu";
font-weight: 400;

Comment 11

[:Gijs (he/him)]

(CC-ing mstange because he gave me ideas here)

Many thanks to Théo and Mark for their valiant efforts in helping me debug this. Here's what happens:

1) through various means, the font size ends up being 11.76666666px.

2) we measure the panel width in em: 22.35em, to be precise

3) we measure the button width relative to that: 22.35em / 3, ie 7.45em.

Now... with this particular font size, that means the perfect values for these widths would be:

11.766666... * 22.35 = 262.98499
11.766666... *  7.45 =  87.66166

87.66166 * 3 is roughly 262.98498. So hurray, math works, our universe is consistent, up is still up and down is still down.

Unfortunately, then CSS rounding kicks in, namely to the nearest 1/60th of a px.

Which means that DOMI's "computed style" view shows that the buttons' width is 87.666666, and the panel's width is 262.98333. In other words, 87 and 40/60th of a pixel, versus 262 and 59/60th of a pixel.

Some more maths tells us that 87 + 40/60 comes to 263 precisely. Which is 1/60th of a pixel too wide, so layout decides to reflow the element to the next line, and we're all very, very sad.

Now, we could change the panel width in the hopes of finding a value that doesn't hit this unfortunate issue. The problem is that with a multitude of screen resolutions, dpi and font settings, I suspect there'll always be someone who will be unhappy...

... unless we put in a gross hack (suggestion courtesy of Markus) and just reduce the pixel width of the button. By 0.1px. Which won't make any difference visually, but will make layout's math come out right again.

Unless someone has a better idea, I think I'll be implementing this after I've caught up on some sleep.

Comment 12

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #11)
> Some more maths tells us that 87 + 40/60 comes to 263 precisely.

Errrrr... (87 + 40/60) * 3 comes to 263 precisely.

Comment 13

[:Gijs (he/him)]

(I didn't get to this today, but I really should tomorrow - bug 897496 would have conflicted with this)

Comment 14

[:Gijs (he/him)]

Attachment: fix Australis panel layout on non-default dpi settings which cause CSS rounding errors,

This is, like, the ugliest thing ever. But it works.

Comment 15

[:Gijs (he/him)]

Attachment: fix Australis panel layout on non-default dpi settings which cause CSS rounding errors,

Markus correctly pointed out that the braces should probably be in the define here.

Comment 16

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

Comment on attachment 8371415 [details] [diff] [review]
fix Australis panel layout on non-default dpi settings which cause CSS rounding errors,

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

Heh, yep, pretty bodacious hackery going on here, but if Markus and Layout thinks this is the best way to address this, then I guess let's roll with it.

Thanks Gijs!

Comment 17

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #16)
> Comment on attachment 8371415 [details] [diff] [review]
> fix Australis panel layout on non-default dpi settings which cause CSS
> rounding errors,
> 
> Review of attachment 8371415 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Heh, yep, pretty bodacious hackery going on here, but if Markus and Layout
> thinks this is the best way to address this, then I guess let's roll with it.
> 
> Thanks Gijs!

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Mark Hammond [:markh] [:mhammond]]

Thanks Gijs - latest nightly WFM

Comment 20

[Mark Hammond [:markh] [:mhammond]]

Gijs, how do you feel about nominating this for Aurora?

Comment 21

[Jim Jeffery not reading bug-mail 1/2/11]

Attachment: menusquished.jpg

Not sure if this bug is responsible, but on my win7 x64 with non-standard DPI, i.e. 125 % the menu now looks squished, History, and Options Icons are flattened, and look frankly, bad ! 

See attached image.

Comment 22

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #21)
> Created attachment 8371928 [details]
> menusquished.jpg
> 
> Not sure if this bug is responsible, but on my win7 x64 with non-standard
> DPI, i.e. 125 % the menu now looks squished, History, and Options Icons are
> flattened, and look frankly, bad ! 
> 
> See attached image.

Nevermind, it was the CTR addon- will advise the author...

Comment 23

[:Gijs (he/him)]

Comment on attachment 8371415 [details] [diff] [review]
fix Australis panel layout on non-default dpi settings which cause CSS rounding errors,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: Users with non-default font sizes/DPI settings will have a broken menu panel layout
Testing completed (on m-c, etc.): manual testing on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Doesn't apply cleanly to Aurora. Please provide a branch patch or request approval for any dependencies.

Comment 25

[:Gijs (he/him)]

Yeah, bug 897496 needs to land first. Sorry about that. I'll leave the branch patch needed until that's landed.

Comment 26

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/f909d0a379f0

Comment 27

[Cornel Ionce [:noni] [Hubs QA]]

Verified fixed on Windows 7 64bit, Ubuntu 12.04 and Mac OS X 10.9 using:
- latest Firefox Aurora (build ID: 20140408004001)
- Firefox 29 beta 6 (build ID: 20140407135746).