900072 - Defect - Line wrapping and layout issues in flyout panel localization

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Flyouts, defect, P2)

Description

[Kapranov]

Attachment: Снимок экрана (82).png

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130729175331

Steps to reproduce:

flyoutpanel too narrow, it will be a problem in the localization. I propose to fix:
diff --git a/browser/metro/theme/flyoutpanel.css b/browser/metro/theme/flyoutpanel.css
.flyout-narrow {
  width: 500px;
}
In this case it will be less problems with localized to other languages.
Screenshot of what it looks like is attached

Comment 1

[Kapranov]

Attachment: FIX

Comment 2

[Kapranov]

Comment on attachment 783827 [details] [diff] [review]
FIX

>diff --git a/browser/metro/theme/flyoutpanel.css b/browser/metro/theme/flyoutpanel.css
--- a/browser/metro/theme/flyoutpanel.css
+++ b/browser/metro/theme/flyoutpanel.css
>.flyout-narrow {
>- width: 346px;
>+ width: 500px;
>}

Comment 3

[Kapranov]

Comment on attachment 783827 [details] [diff] [review]
FIX

diff --git a/browser/metro/theme/flyoutpanel.css b/browser/metro/theme/flyoutpanel.css
--- a/browser/metro/theme/flyoutpanel.css
+++ b/browser/metro/theme/flyoutpanel.css

.flyout-close-button:active {
  -moz-image-region: rect(0 96px 32px 64px);
}

.flyout-narrow {
- width: 346px;
+ width: 500px;
}

Comment 4

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Our flyouts need to be one of two standard widths:
  - For narrow flyouts, 346px
  - For wide flyouts, I don't recall the exact number but I think it's 646px

A solution might be to do the following:
  - Add a new css class, flyout-wide, whose width is equal to the wider standard flyout width
  - In JS, when showing a flyout for the first time, check whether the content of the flyout requires the class to change from flyout-narrow to flyout-wide
  - If the content is too wide even for a flyout-wide, additionally make the flyout scrollable in the x direction

Comment 5

[Kapranov]

Scroll in the direction of x - is not an option, it is too long it turns out.

Comment 6

[Jim Mathies [:jimm]]

Text should wrap in these panels. Once bug 898457 lands lets recheck this.

Comment 7

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Text wrapping helps for long sentences, but it doesn't help if a single word is wider than the whole flyout. In those cases we'll need to switch to the wider flyout width.

Comment 8

[Kapranov]

Breaking lines for some reason do not work everywhere, if it was - it would be all very well, though the panel might be still a little wider so that the switches were cleaned and closed the scroll bar

Comment 9

[Asa Dotzler [:asa]]

I don't think we should make the panels nonstandard sizes. We should wrap when we can and elide or truncate when we cannot.

Comment 10

[Matt Brubeck (:mbrubeck)]

Word wrapping should be working in these panels now; is there still a bug here in some locales?

Would it help if we enabled hyphenation using "-moz-hyphens: auto"?
https://developer.mozilla.org/en-US/docs/Web/CSS/hyphens

Comment 11

[Jim Mathies [:jimm]]

bug 906329 has some good issue screenshots.

Comment 12

[Kapranov]

Attachment: Снимок экрана (133).png

Comment 13

[Kapranov]

Attachment: Снимок экрана (134).png

Comment 15

[Matt Brubeck (:mbrubeck)]

Taking this for the current iteration; work estimate p=3

Comment 16

[Kapranov]

About panel
To update channel name may be to add a command, such as brandShortName, and for the line about politics is kept strictly confidential to the lines 2 and 3, to be used when needed. Well, or wrapping is done ..

Comment 17

[Francesco Lodolo [:flod]]

Any progress on this? My fear is that too much wrapping will cause problems with the vertical space available, in particular with the Options panel.

Comment 18

[Marco Mucci [:MarcoM]]

Hi Flod, Bug 900072 is currently being worked on this iteration.

Comment 19

[Matt Brubeck (:mbrubeck)]

Attachment: part 1: force wrapping

There's definitely something weird going on with the layout here.  For example, in the About flyout, setting a max-width on any one of the children causes all of them to have the correct width, for no reason I can see.  In this patch I just add a <spacer> child to force the width, since this should be a little more robust and performant than the previous workaround.

The other misbehaving elements were the <radio> buttons in the Options panel.  Again, I just had to hard-code a max-width for them.

This is a hacky workaround; I'm not super happy with it and will try to dig deeper into the layout issue.

We may still have issues with e.g. the toggle switches in locales where their labels are long; I'll deal with that in a separate patch.

Comment 20

[Matt Brubeck (:mbrubeck)]

Attachment: part 1: force wrapping (v2)

Forgot to qrefresh.

Comment 21

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 809393 [details] [diff] [review]
part 1: force wrapping (v2)

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

No better ideas here either. It does appear the fix the problem, so that's progress.

Comment 22

[Kapranov]

Attachment: Снимок экрана (56).png

Excellent!

Comment 23

[Matt Brubeck (:mbrubeck)]

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

Comment 24

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

https://hg.mozilla.org/mozilla-central/rev/851e6aed858a

Comment 25

[Marco Mucci [:MarcoM]]

Temporarily reopening to add to iteration.  Will mark as resolved right after.

Comment 26

[Francesco Lodolo [:flod]]

Attachment: Screenshot

There is still an issue with the Remember password option.

Maybe a silly question, but wouldn't it be easier to have the switches on a different line? Below/above the text, or together with sections' titles? Having localizations with different lengths for on/off cause the text to move around unpleasantly.

Comment 27

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #26)
> Created attachment 810717 [details]
> There is still an issue with the Remember password option.

Sorry for the noise. Apparently I didn't have the latest update, with 2013-09-26 that part is fixed.

> Maybe a silly question, but wouldn't it be easier to have the switches on a
> different line? Below/above the text, or together with sections' titles?
> Having localizations with different lengths for on/off cause the text to
> move around unpleasantly.

This remains valid though.

Comment 28

[Matt Brubeck (:mbrubeck)]

(In reply to Francesco Lodolo [:flod] from comment #26)
> Maybe a silly question, but wouldn't it be easier to have the switches on a
> different line? Below/above the text, or together with sections' titles?
> Having localizations with different lengths for on/off cause the text to
> move around unpleasantly.

I agree.  I think we should make two changes:

1. Make toggle switches keep a constant width (large enough to contain the longest label) so they don't cause other elements to move when you toggle them.

2. Put the toggle switch on a separate line, at least  if it's over a certain width.

Comment 29

[Matt Brubeck (:mbrubeck)]

Comment on attachment 809393 [details] [diff] [review]
part 1: force wrapping (v2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug in Metro UI (not a regression).

User impact if declined: Bad layout in Metro options flyout for some locales.

Testing completed (on m-c, etc.): Landed on m-c on 2013-09-25.

Risk to taking this patch (and alternatives if risky): Very low risk metro-only patch.

String or IDL/UUID changes made by this patch: None.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/37c5080673ea

Comment 31

[Manuela Muntean [Away]]

Attachment: screenshot_Nightly_it build.png

While testing this for iteration #16:

- with the Italian Nightly build from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/10/2013-10-16-03-02-02-mozilla-central-l10n/ I can still see some layout issues with the About flyout (please see the attached screenshot)

Does anyone have any thoughts/suggestions? Thanks!

Comment 32

[Manuela Muntean [Away]]

> While testing this for iteration #16:
> 
> - with the Italian Nightly build from
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/10/2013-10-16-03-
> 02-02-mozilla-central-l10n/ I can still see some layout issues with the
> About flyout (please see the attached screenshot)

Forgot to mention the environment: Win 8 32-bit

> Does anyone have any thoughts/suggestions? Thanks!

Comment 33

[Francesco Lodolo [:flod]]

> Does anyone have any thoughts/suggestions? Thanks!

I guess that would be bug 926036

Comment 34

[Manuela Muntean [Away]]

I've also checked this with the build from comment 31 on Win 8 64-bit, but also with Aurora in Metro mode, with the Italian build from: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/10/2013-10-17-00-40-02-mozilla-aurora-l10n/ , with both Win 8 32-bit and 64-bit, and everything looks fine, except for the About flyout that looks the same as https://bugzilla.mozilla.org/attachment.cgi?id=818446

I'm marking this as verified, for iteration #16, based on comment 31 and 34, and based on the fact that the issue with the About flyout is tracked in bug 926036.