Closed Bug 900072 Opened 7 years ago Closed 6 years ago

Defect - Line wrapping and layout issues in flyout panel localization

Categories

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

25 Branch
All
Windows 8
defect

Tracking

(firefox26 fixed, firefox27 fixed)

VERIFIED FIXED
Firefox 27
Tracking Status
firefox26 --- fixed
firefox27 --- fixed

People

(Reporter: kapranov98, Assigned: mbrubeck)

References

Details

(Keywords: intl, Whiteboard: [l10n] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3)

Attachments

(7 files, 2 obsolete files)

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
OS: All → Windows 8
Attached patch FIX (obsolete) — Splinter Review
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 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;
}
Summary: The narrow panel flyoutpanel → Defect - The narrow panel flyoutpanel
Whiteboard: feature=defect c=tbd u=tbd p=0
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
Scroll in the direction of x - is not an option, it is too long it turns out.
Text should wrap in these panels. Once bug 898457 lands lets recheck this.
Depends on: 898457
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.
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
Summary: Defect - The narrow panel flyoutpanel → Defect - The standard narrow panel flyoutpanel might not work for all locales
Whiteboard: feature=defect c=tbd u=tbd p=0 → feature=defect c=tbd u=tbd p=0 [preview-triage]
Priority: -- → P2
I don't think we should make the panels nonstandard sizes. We should wrap when we can and elide or truncate when we cannot.
Blocks: 831955
Whiteboard: feature=defect c=tbd u=tbd p=0 [preview-triage] → feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0 [preview-triage]
Whiteboard: feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0 [preview-triage] → [preview-triage] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0
Blocks: metrov1backlog
No longer blocks: metrov1defect&change
Summary: Defect - The standard narrow panel flyoutpanel might not work for all locales → [MP] Defect - The standard narrow panel flyoutpanel might not work for all locales
Whiteboard: [preview-triage] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0 → [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0
Summary: [MP] Defect - The standard narrow panel flyoutpanel might not work for all locales → [MP] Defect - Make sure that line wrap & eliding work in nonenglish locales on flyouts
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
Flags: needinfo?(kapranov98)
Status: UNCONFIRMED → NEW
Ever confirmed: true
bug 906329 has some good issue screenshots.
Depends on: 906329
Flags: needinfo?(kapranov98)
Duplicate of this bug: 906329
Taking this for the current iteration; work estimate p=3
Assignee: nobody → mbrubeck
Blocks: 852263
Keywords: intl
Summary: [MP] Defect - Make sure that line wrap & eliding work in nonenglish locales on flyouts → [MP] Defect - Line wrapping and layout issues in flyoutpanel localization
Whiteboard: [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=0 → [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3
Blocks: metrov1it13
No longer blocks: metrov1backlog
Status: NEW → ASSIGNED
QA Contact: jbecerra
Depends on: 908179
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 ..
Blocks: metrov1it14
No longer blocks: metrov1it13
Whiteboard: [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3 → [l10n] [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3
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.
Hi Flod, Bug 900072 is currently being worked on this iteration.
Blocks: metrov1it15
No longer blocks: metrov1it14
Attached patch part 1: force wrapping (obsolete) — Splinter Review
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.
Attachment #783827 - Attachment is obsolete: true
Attachment #809386 - Flags: review?(sfoster)
Forgot to qrefresh.
Attachment #809386 - Attachment is obsolete: true
Attachment #809386 - Flags: review?(sfoster)
Attachment #809393 - Flags: review?(sfoster)
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.
Attachment #809393 - Flags: review?(sfoster) → review+
Excellent!
https://hg.mozilla.org/mozilla-central/rev/851e6aed858a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Blocks: metrov1it16
No longer blocks: MetroPreviewRelease, metrov1it15
Summary: [MP] Defect - Line wrapping and layout issues in flyoutpanel localization → Defect - Line wrapping and layout issues in flyout panel localization
Whiteboard: [l10n] [preview] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3 → [l10n] feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=3
Temporarily reopening to add to iteration.  Will mark as resolved right after.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Attached image 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.
(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.
(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 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.
Attachment #809393 - Flags: approval-mozilla-aurora?
Attachment #809393 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 926036
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!
Flags: needinfo?(mbrubeck)
> 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!
> Does anyone have any thoughts/suggestions? Thanks!

I guess that would be bug 926036
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.
Status: RESOLVED → VERIFIED
Flags: needinfo?(mbrubeck)
You need to log in before you can comment on or make changes to this bug.