Closed Bug 1172406 Opened 9 years ago Closed 9 years ago

There's no way to set up a new Master Password on Linux with some locales

Categories

(Firefox :: Settings UI, defect)

Unspecified
Linux
defect
Not set
normal
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.3 - Jun 29
Tracking Status
firefox38 --- wontfix
firefox39 + verified
firefox38.0.5 --- wontfix
firefox40 + verified
firefox41 + verified

People

(Reporter: theo, Assigned: jaws)

Details

(Keywords: regression)

Attachments

(3 files, 2 obsolete files)

On Linux, using French build (I guess other locales using long strings are also affected), when a user wants to set up a master password, there's no "OK" button and the text on the panel is cut off.

The critical part is the missing button. Since there's only a "Cancel" button, the whole feature is unusable.

Windows builds does not seem affected. I did not test on Mac OS.
The issue did not occur with the old preferences panel.

I'm guessing the string "Saisissez le nouveau mot de passe :" is responsible for the overflow. I can probably shorten it as "Nouveau mot de passe :", but something should be done code side to prevent this issue from happening in the first place. And to fix all locales.

Tracking requested, because a feature is broken.
Tracked for 39, 40, and 41, because it is unusable for Linux users.
This may be from in-content preferences changes in 38.  We should try to fix this for 39 if possible.
Flags: needinfo?(jaws)
Adding Gijs to the needinfo list (which already includes me) to make sure that we get a pair of eyes on this soon so we can get it fixed in one of the last betas for 39.
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Iteration: --- → 41.3 - Jun 29
Points: --- → 2
Flags: qe-verify+
Flags: needinfo?(jaws)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: in-testsuite-
Flags: firefox-backlog+
Attached patch Patch (obsolete) — Splinter Review
Also fixed some trailing whitespace while I was in here.
Attachment #8622019 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8622019 [details] [diff] [review]
Patch

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

Interesting. It's sad we need both of these - my experiences with flex + wrapping text aren't great (oh hi, forget button panel and friends!).

Also, it seems to me like the "proper" fix would be to make the width: 35em; at the top of the file localizable (or to get rid of it entirely). I recognize that that can't be uplifted, but that seems better. For uplift to aurora, I guess this patch would work (minus the whitespace changes if necessary to avoid conflicts). Personally, I think it's pretty late in beta to try to do something about that here considering we shipped 38 with it and the world hasn't ended. Keyboard shortcuts will still work, and esp. on Linux I expect people to know those (enter/esc to confirm/cancel dialogs).

Finally, I can reproduce the original issue with en-US in that content gets cut off; the fix causes all the textbox labels to wrap, at least on Linux, which isn't ideal.

Tentative r+, but please consider the above?
Attachment #8622019 - Flags: review?(gijskruitbosch+bugs) → review+
Attached patch Patch for beta (obsolete) — Splinter Review
We could do this for beta.

I would like to use the other patch on Aurora and Nightly, and figure out why that patch is causing the text to wrap. That other patch can probably remove this width altogether, and we can make this dialog resizable.
Attachment #8623845 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8623845 [details] [diff] [review]
Patch for beta

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

This is not wide enough for French on my Ubuntu VM. 50em works but I don't know if that is likely to lead to other problems (e.g. small screens and large font sizes). Rock, hard place. What do you think is safest for beta? (time is running out for this beta, sadly - maybe the safest for 39 is to continue shipping what we have been on 38? :-\ )
Attachment #8623845 - Flags: review?(gijskruitbosch+bugs) → review-
I think we should go with the first patch in this bug for beta, but need to figure out why I see no wrapping and you do see wrapping. Can you try the build from https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc0ebce9053c ?
Flags: needinfo?(gijskruitbosch+bugs)
Attached patch Patch v1.1Splinter Review
This combines both patches and uses 40em for the default width of the dialog. Gijs confirmed over IRC that the new width prevents wrapping on en-US.
Attachment #8622019 - Attachment is obsolete: true
Attachment #8623845 - Attachment is obsolete: true
Attachment #8624306 - Flags: review?(gijskruitbosch+bugs)
Attachment #8624306 - Flags: review?(gijskruitbosch+bugs) → review+
Flags: needinfo?(gijskruitbosch+bugs)
Approval Request Comment
[Feature/regressing bug #]: in-content preferences (released in v38)
[User impact if declined]: some users are unable to set Master Password due to dialog being too small
[Describe test coverage new/current, TreeHerder]: manual testing on multiple platforms
[Risks and why]: low risk, increasing the width of the dialog and allowing wrapping of text if it is too long
[String/UUID change made/needed]: none
Attachment #8624321 - Flags: approval-mozilla-beta?
Attachment #8624321 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7cab8e8734c7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
QA Contact: kjozwiak
Comment on attachment 8624321 [details] [diff] [review]
Patch for Aurora40 and Beta39 (less whitespace fixes)

Regression and low risk, taking it for aurora.
Attachment #8624321 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8624321 [details] [diff] [review]
Patch for Aurora40 and Beta39 (less whitespace fixes)

OK to uplift to beta for the RC.
Attachment #8624321 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Went through verification using the following builds:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-07-03-03-02-16-mozilla-central-l10n/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-07-03-00-40-18-mozilla-aurora-l10n/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/39.0/

Used the following OS's:

- Ubuntu 14.04.2 x64 - PASSED
- Win 8.1 x64 - PASSED

Test Cases Used: (used fr & de locales on each channel/OS)

- selected about:preferences#security
- enabled master password and clicked on "Change Master Password.."
- ensured that the "OK" button wasn't being cut off in the window
- changed the master password several times and ensured that the "OK" button appeared every time

Found Bug # 1180281 while going through the verification process in this bug.
You need to log in before you can comment on or make changes to this bug.