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)
Tracking
()
People
(Reporter: theo, Assigned: jaws)
Details
(Keywords: regression)
Attachments
(3 files, 2 obsolete files)
68.06 KB,
image/png
|
Details | |
3.45 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
2.78 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Comment 1•9 years ago
|
||
Tracked for 39, 40, and 41, because it is unusable for Linux users.
Comment 2•9 years ago
|
||
This may be from in-content preferences changes in 38. We should try to fix this for 39 if possible.
Flags: needinfo?(jaws)
Assignee | ||
Comment 3•9 years ago
|
||
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 | ||
Updated•9 years ago
|
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+
Assignee | ||
Comment 4•9 years ago
|
||
Also fixed some trailing whitespace while I was in here.
Attachment #8622019 -
Flags: review?(gijskruitbosch+bugs)
Comment 5•9 years ago
|
||
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+
Assignee | ||
Comment 6•9 years ago
|
||
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 7•9 years ago
|
||
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-
Assignee | ||
Comment 8•9 years ago
|
||
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)
Assignee | ||
Comment 9•9 years ago
|
||
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)
Updated•9 years ago
|
Attachment #8624306 -
Flags: review?(gijskruitbosch+bugs) → review+
Updated•9 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 11•9 years ago
|
||
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?
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7cab8e8734c7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Updated•9 years ago
|
QA Contact: kjozwiak
Updated•9 years ago
|
Comment 13•9 years ago
|
||
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 14•9 years ago
|
||
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+
Comment 17•9 years ago
|
||
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.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•