987586 - "Customize Mode" doesn't work well if strings for customization option buttons are long (due to l10n or font size)

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[kestkonstantin]

Attachment: Notice how GUI got cropped in the right side of the window.

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140318013849

Steps to reproduce:

1. Launch Firefox with new UI on monitor with 1024x768 pixels, fullscreen or windowed.
2. Open main menu and click 'Customize'.


Actual results:

Customization menu doesn't get on the screen and cropped on the right side.


Expected results:

Interface must resize to fit inside window frame.

Comment 1

[:Gijs (he/him)]

Ugh, that looks terrible. This is because the buttons at the bottom can't wrap, and is in part due to the localization you're using having strings which are more than twice as long as the English ones. :-(

We should probably look if we can somehow wrap the buttons, and we should alert the localization team for the locale that you're using. Can you let us know which language that is? (I'm aware there are various languages which use Cyrillic script and I am not familiar enough with it to know which one this is, sorry!)

Comment 2

[Francesco Lodolo [:flod]]

Based on the subscribe button, should be Russian
http://transvision.mozfr.org/string/?entity=browser/chrome/browser/customizableui/customizableWidgets.properties:feed-button.label&repo=central

Comment 3

[:Gijs (he/him)]

Right, so this is the Russian translation, and they already changed these strings, so the problem should be a lot less severe in upcoming versions of beta:

https://hg.mozilla.org/l10n-central/ru/rev/8129f180f0b7

We should probably still investigate wrapping these buttons, but it's not as easy as I hoped it would be. :-(

Comment 4

[Alexander L. Slovesnik]

(In reply to :Gijs Kruitbosch from comment #3)
> Right, so this is the Russian translation, and they already changed these
> strings, so the problem should be a lot less severe in upcoming versions of
> beta:
> 
> https://hg.mozilla.org/l10n-central/ru/rev/8129f180f0b7
> 
Yes, it's been reported on Russian forum on http://forum.mozilla-russia.org/viewtopic.php?id=62329 and I've made these strings shorter.
Here is screenshot of Aurora with new strings on 800x600 - http://itmages.ru/image/view/1567419/03dd3655

Comment 6

[[:Aleksej]]

Were those screenshots made in full-screen mode, or windowed mode?  For me, 2014-05-04-03-02-05-mozilla-central-firefox-32.0a1.he.linux-x86_64 enlarges the window if it is small, so the UI fits if there is enough space on the screen (but it does not resize back when exiting Customize Mode).

Comment 8

[:Gijs (he/him)]

(In reply to [:Aleksej] from comment #6)
> Were those screenshots made in full-screen mode, or windowed mode?  For me,
> 2014-05-04-03-02-05-mozilla-central-firefox-32.0a1.he.linux-x86_64 enlarges
> the window if it is small, so the UI fits if there is enough space on the
> screen (but it does not resize back when exiting Customize Mode).

This is expected, but we should still try to make the buttons wrap so they fit on smaller screens.

Comment 9

[:Gijs (he/him)]

Attachment: flex and wrap customization footer,

Comment 10

[:Gijs (he/him)]

Grrrr... bzexport seems to be broken for me. Anyway, my comment as it was meant to submit:

So this was fun... To get stuff to wrap, I needed to switch to non-XUL display style (block or flex). For the reset button to be end-aligned, I needed a flexible space (using just float: right/left looked weird once stuff started wrapping, because the other buttons were still aligned to the other side).

For the flexible space to work I needed to use display: flex and flex-wrap: wrap. When wrapping, that wasn't increasing the height of the footer, which I fixed by making the entire container be display: flex... Everything seems to work, but yeah, hence the extensive changes.

The margins don't collapse (presumably because of XUL frames?) and so they are carefully selected in order to maintain the current style (15px total spacing between button edge and footer edge, 10px spacing between each button, both horizontally and vertically).

Finally, I was seeing odd vertical misalignment, which I fixed by using vertical-align: middle for all the buttons and the image in the titlebar button.

Comment 11

[:Gijs (he/him)]

Marco, can you add this to this iteration, please?

Comment 12

[Marco Mucci [:MarcoM]]

Added to Iteration 33.2

Comment 13

[:Gijs (he/him)]

Comment on attachment 8449280 [details] [diff] [review]
flex and wrap customization footer,

Per discussion on IRC, ui-review?phlsa :-)

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8449280 [details] [diff] [review]
flex and wrap customization footer,

> .customizationmode-button {
>+  min-height: 30px;

What is this good for?

Comment 15

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #14)
> Comment on attachment 8449280 [details] [diff] [review]
> flex and wrap customization footer,
> 
> > .customizationmode-button {
> >+  min-height: 30px;
> 
> What is this good for?

The titlebar button has a 24px image, plus 2 * (2px padding + 1px border) == 30px high. When not using a XUL-displayed container, the other buttons shrunk.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Even with the min-height: 30px, on Windows 7 Aero with the buttons each on their own row:
#customization-titlebar-visibility-button: box height=35
#customization-toolbar-visibility-button: box height=30
#customization-undo-reset-button: box height=30
#customization-reset-button: box height=30

The extra 5 pixels for the #customization-titlebar-visibility-button comes from the .button-box child inside of it that has:
> border-top-width: 1px;
> border-bottom-width: 1px;
> padding-top: 1px;
> padding-bottom: 2px;

I only see the other buttons shrink when the buttons wrap to their own line. When they are all displayed on the same line then they stretch vertically to match the height of the tallest button.

So I don't see the min-height:30px as being the right fix here. We could either do nothing, and let the buttons shrink (which might not be so noticeable), or more work could be done to get the proper min-height for each respective environment.

Comment 17

[:Gijs (he/him)]

Attachment: flex and wrap customization footer,

This fixes the height of the icon instead, which should work.

Comment 18

[:Gijs (he/him)]

Attachment: flex and wrap customization footer,

With flex-shrink on the footer per IRC discussion with jaws.

Comment 19

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Comment on attachment 8449533 [details] [diff] [review]
flex and wrap customization footer,

Looks great!

Comment 20

[:Gijs (he/him)]

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

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/80b59bea12f8

Comment 22

[Marco Mucci [:MarcoM]]

Hi Florin, can a QA contact be assigned for verification.

Comment 23

[:Gijs (he/him)]

Considering the impact and number of duplicates, I'd actually quite like this to make 32. However, I would feel bad asking for uplift before verification... what are my odds of verification happening in the next two weeks? (so this can make the first 32 beta) :-)

Comment 24

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

Mozilla/5.0 (Windows NT 6.1; rv:33.0) Gecko/20100101 Firefox/33.0

Reproduced the issue on Firefox 30 release with the Russian and Arabic locales.

On latest Nightly (build ID: 20140708030203), the "Customize mode" is properly displayed, without being cropped using 1024x768 and 800x600 resolutions on the mentioned locales.
Marking this verified.

Comment 25

[:Gijs (he/him)]

Comment on attachment 8449533 [details] [diff] [review]
flex and wrap customization footer,

(In reply to Cornel Ionce [QA] from comment #24)
> Mozilla/5.0 (Windows NT 6.1; rv:33.0) Gecko/20100101 Firefox/33.0
> 
> Reproduced the issue on Firefox 30 release with the Russian and Arabic
> locales.
> 
> On latest Nightly (build ID: 20140708030203), the "Customize mode" is
> properly displayed, without being cropped using 1024x768 and 800x600
> resolutions on the mentioned locales.
> Marking this verified.

Sweet, thank you!


Although I would like this on beta and 31-esr, there's only one beta left, and I'm loathe to take the risk at this point, which is unfortunate because we'll have to live with the existing situation for 31. :-\

In any case, 32 should be fine:

Approval Request Comment
[Feature/regressing bug #]: Australis
[User impact if declined]: in some locales / screen sizes, you can't use customize mode very well
[Describe test coverage new/current, TBPL]: styling only, so no direct test coverage
[Risks and why]: relatively low because mostly CSS and minor XUL changes
[String/UUID change made/needed]: none

Comment 26

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8449533 [details] [diff] [review]
flex and wrap customization footer,

Good catch on this one. I also wish we caught this a bit earlier so that we could have taken it on 31. Alas. Aurora+ it is.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

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

Comment 28

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

Verified fixed on Firefox 32 beta 6, build ID: 20140811180644 and latest Aurora, build ID: 20140812004001 using the mentioned locales.