Closed Bug 1589652 Opened 10 months ago Closed 10 months ago

Long sync engine choice translations cause issues about:preferences#sync and “Choose what to sync” dialog

Categories

(Firefox :: Preferences, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 72
Tracking Status
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 + fixed
firefox72 --- fixed

People

(Reporter: tchevalier, Assigned: markh)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Even when the panel is wide enough, the label keeps wrapping

[Tracking Requested - why for this release]:
Recent user-visible regression

Keywords: regression
Regressed by: 1570567

Mark, this is another one, I imagine the fix will be similar / the same as bug 1589651, but fyi...

Flags: needinfo?(markh)
Priority: -- → P1
See Also: → 1589651

Mark is working on this now.

Assignee: nobody → markh
Duplicate of this bug: 1589651
Status: NEW → ASSIGNED
Flags: needinfo?(markh)
Summary: “Logins and passwords” label in “Choose what to sync” panel shouldn’t wrap → Long sync engine choice translations cause issues about:preferences#sync and “Choose what to sync” dialog
Attached image long-engine-names.png

This is what the patch I'm about to upload looks like with long engine names (although each part of the image is scaled differently) - obviously though I'm not going to include the changes to the strings or the width I used for testing.

  • In about:preferences#sync, we don't really have the ability to widen the area used to display the names - so at some point we are still forced to wrap. However, not all the available space was used before we did wrap, and I've resolved that (ie, the labels now get much closer to the middle/end before wrapping). The patch also ensures that the icon and the label are not split, which is also what we saw before. I think even with the bug as reported, the translated string will fit without any wrapping, so this should be considered a "worst case".

  • In the "choose what to sync" dialog we do have the ability to increase the width of the dialog - and this can be done in the .ftl file, so it can be different for each translation. In the screenshot attached, the .ftl specifies the width should be 60em instead of the 36em used by the EN/default .ftl file. Note that I lost my battle with CSS, so I couldn't arrange for it to also wrap if the specified width wasn't wide enough - so an overly long label will "bleed" into the second column if the width is too small - but the solution there is simply to adjust the width in that translation's .ftl file.

FTR, part of the .ftl patch I used for testing included:

@@ -798,32 +798,32 @@ sync-change-options =

 sync-choose-what-to-sync-dialog =
     .title = Choose What To Sync
-    .style = width: 36em; min-height: 35em;
+    .style = width: 60em; min-height: 35em;
     .buttonlabelaccept = Save Changes
Attachment #9103833 - Attachment description: Bug 1589652 - better handling of long sync engine names in the preferences UI. r?Gijs,flod → Bug 1589652 - better handling of long sync engine names in the preferences UI. r?Gijs
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/da926e5cf009
better handling of long sync engine names in the preferences UI. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

Mark, this bug is a P1 and the fix is CSS only, do you want to request an uplift to beta? Thanks

Flags: needinfo?(markh)

Comment on attachment 9103833 [details]
Bug 1589652 - better handling of long sync engine names in the preferences UI. r?Gijs

Beta/Release Uplift Approval Request

  • User impact if declined: Visually ugly UI for some non-English users
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): CSS only change, impact limited to sync preferences.
  • String changes made/needed: None
Flags: needinfo?(markh)
Attachment #9103833 - Flags: approval-mozilla-beta?

Comment on attachment 9103833 [details]
Bug 1589652 - better handling of long sync engine names in the preferences UI. r?Gijs

71 regression, visual fix for non-English users, CSS only and low risk, uplift approved for 71 beta 6, thanks.

Attachment #9103833 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

For the record, this cause the below regression, but was fixed 4 days later.
== Change summary for alert #23716 (as of Tue, 05 Nov 2019 11:06:55 GMT) ==

Regressions:

6% JS linux64-shippable opt stylo tp6 194,898,781.08 -> 206,764,398.62
3% JS linux64-shippable opt stylo tp6 202,861,674.92 -> 208,933,206.16

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=23716

(In reply to Alexandru Ionescu :alexandrui from comment #14)

For the record, this cause the below regression, but was fixed 4 days later.
== Change summary for alert #23716 (as of Tue, 05 Nov 2019 11:06:55 GMT) ==

Regressions:

6% JS linux64-shippable opt stylo tp6 194,898,781.08 -> 206,764,398.62
3% JS linux64-shippable opt stylo tp6 202,861,674.92 -> 208,933,206.16

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=23716

This genuinely doesn't make any sense - the only changes were to files not opened in this test. You're probably mis-attributing these from some other changeset.

Flags: needinfo?(alexandru.ionescu)

(In reply to :Gijs (he/him) from comment #15)

This genuinely doesn't make any sense - the only changes were to files not opened in this test. You're probably mis-attributing these from some other changeset.

Did some retriggers to confirm that.

Flags: needinfo?(alexandru.ionescu)

(In reply to Alexandru Ionescu :alexandrui from comment #16)

(In reply to :Gijs (he/him) from comment #15)

This genuinely doesn't make any sense - the only changes were to files not opened in this test. You're probably mis-attributing these from some other changeset.

Did some retriggers to confirm that.

Can we get a bug on file to figure out why the test values were influenced by this change? It doesn't make a lot of sense, and suggests the test is bogus.

Flags: needinfo?(alexandru.ionescu)

Done that.

Flags: needinfo?(alexandru.ionescu)
You need to log in before you can comment on or make changes to this bug.