kestkonstantin Reporter
	Description • 10 years ago

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.

	:Gijs (he/him) Assignee
	Comment 1 • 10 years ago

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!)

	Francesco Lodolo [:flod]
	Comment 2 • 10 years ago

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

	:Gijs (he/him) Assignee
	Comment 3 • 10 years ago

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. :-(

	Alexander L. Slovesnik
	Comment 4 • 10 years ago

(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

	[:Aleksej]
	Comment 6 • 10 years ago

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).

	:Gijs (he/him) Assignee
	Comment 8 • 10 years ago

(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.

	:Gijs (he/him) Assignee
	Comment 10 • 10 years ago

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.

	:Gijs (he/him) Assignee
	Comment 11 • 10 years ago

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

	Marco Mucci [:MarcoM]
	Comment 12 • 10 years ago

Added to Iteration 33.2

	:Gijs (he/him) Assignee
	Comment 13 • 10 years ago

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

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

	Dão Gottwald [:dao]
	Comment 14 • 10 years ago

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

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

What is this good for?

	:Gijs (he/him) Assignee
	Comment 15 • 10 years ago

(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.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 16 • 10 years ago

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.

	:Gijs (he/him) Assignee
	Comment 17 • 10 years ago

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

	:Gijs (he/him) Assignee
	Comment 18 • 10 years ago

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

	(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo
	Comment 19 • 10 years ago

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

Looks great!

	:Gijs (he/him) Assignee
	Comment 20 • 10 years ago

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

	Carsten Book [:Tomcat]
	Comment 21 • 10 years ago

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

	Marco Mucci [:MarcoM]
	Comment 22 • 10 years ago

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

	:Gijs (he/him) Assignee
	Comment 23 • 10 years ago

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) :-)

	Cornel Ionce [:noni] [Hubs QA]
	Comment 24 • 10 years ago

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.

	:Gijs (he/him) Assignee
	Comment 25 • 10 years ago

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

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 26 • 10 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 27 • 10 years ago

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

	Cornel Ionce [:noni] [Hubs QA]
	Comment 28 • 10 years ago

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