Closed
Bug 927785
Opened 11 years ago
Closed 11 years ago
Let labels use all the available space (reduce paddings)
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Firefox OS Graveyard
Gaia::Settings
Tracking
(blocking-b2g:koi+, b2g-v1.2 fixed)
People
(Reporter: flod, Assigned: kaze)
References
Details
(Keywords: regression)
Attachments
(6 files)
Not a great summary, but I couldn't find a better one.
In the attached screenshot you can see the same string used in the same context on 1.1 and 1.2: on 1.2 the string is automatically cut (I already had to use an abbreviation in 1.1) without any reasonable explanation.
"cc" is replaced with "...", and the amount of blank space between label and checkbox is just preposterous for locales who have to fight to the last character to fit in ;-)
Comment 1•11 years ago
|
||
asking for UX help about this. thanks
Flags: needinfo?(firefoxos-ux-bugzilla)
Updated•11 years ago
|
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 3•11 years ago
|
||
Yes, the check box was shifted to the right, this opened up more space for the text. The string should have a padding of 30px on the left and right sides. Can the padding be updated? On a side note I opened a bug that proposes to allowed 2 lines in settings when strings can't fit on one line (908309). But I don't think this is relevant for this bug given the available space :). Hope this helps!
Flags: needinfo?(epang)
Comment 4•11 years ago
|
||
Who is the right person to create the patch that adds the padding? Lets use this bug to track that work.
Updated•11 years ago
|
blocking-b2g: --- → leo?
Comment 5•11 years ago
|
||
this should definitely be considered as blocking koi (1.2) since the current state regresses the available space for strings from the previous release and makes our previous string truncation problem worse. The string truncation bug list for 1.2 is long https://bugzilla.mozilla.org/show_bug.cgi?id=928174 and one patch here may help reduce significantly.
Some examples of the effect of button size change without the corresponding padding change are in the newsgroup thread on this topic: https://groups.google.com/forum/#!topic/mozilla.dev.l10n/mBVbvgzfEIg
blocking-b2g: leo? → koi?
Comment 6•11 years ago
|
||
It should be easy to fix on 1.2 because it looks like it has been recently fixed on master, but not uplifted yet.
I couldn't find the bug/changeset, though.
We were massively hit by this bug for French, and now I managed to bring back some of our previous (longer) translations to 1.3, without truncation https://hg.mozilla.org/gaia-l10n/fr/rev/8dce303169f4
Comment 7•11 years ago
|
||
ni? Eric Pang as he seems to have worked on this issue earlier
Flags: needinfo?(epang)
Comment 8•11 years ago
|
||
(In reply to chris hofmann from comment #5)
> this should definitely be considered as blocking koi (1.2) since the current
> state regresses the available space for strings from the previous release
> and makes our previous string truncation problem worse. The string
> truncation bug list for 1.2 is long
> https://bugzilla.mozilla.org/show_bug.cgi?id=928174 and one patch here may
> help reduce significantly.
>
> Some examples of the effect of button size change without the corresponding
> padding change are in the newsgroup thread on this topic:
> https://groups.google.com/forum/#!topic/mozilla.dev.l10n/mBVbvgzfEIg
Looks like comment 6 has some l10n alternatives to resolve this in l10n branch. So not sure we still need to consider blocking on this.Can you confirm if that helps ? Else we may have to involve UX here.
Comment 9•11 years ago
|
||
I think the solution in comment 6 is exactly what needs to be done. uplift the patch from master for adding padding as the button size has already been reduced in both places. Its the lack of having both of those fixes landed together that has increased the truncation problem in 1.2
We can either get the padding change uplifted or have a large and undefined number of truncation that we will need to address individually.
Either way we should make the changes before the next round of l10n testing starts for 1.2 to avoid wasting test time and testing cycles.
Lets do the more simple solution suggested in comment 6. That involves finding the patch on master, reviewing, and getting it landed for 1.2
Comment 10•11 years ago
|
||
Taipei triage result = minus it, because it looks like a minor issue.
blocking-b2g: koi? → -
Reporter | ||
Comment 11•11 years ago
|
||
(In reply to Kevin Hu [:khu] from comment #10)
> Taipei triage result = minus it, because it looks like a minor issue.
Could you please define "minor"? It practically impacts every single language except for en-US and languages shorter than en-US (e.g. Chinese).
And the main problem is that a string that was good in 1.1 and will be good in 1.3, it's not good in 1.2, causing a massive amount of unnecessary QA bugs for truncation and low quality localized builds.
Flags: needinfo?(khu)
Comment 12•11 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #11)
> Could you please define "minor"? It practically impacts every single
> language except for en-US and languages shorter than en-US (e.g. Chinese).
>
Minus means not to fix in the corresponding version(in this case, it means 1.2, because it was marked as koi?).
> And the main problem is that a string that was good in 1.1 and will be good
> in 1.3, it's not good in 1.2, causing a massive amount of unnecessary QA
> bugs for truncation and low quality localized builds.
Thank you for your feedback. In this case, let's try to fix it in 1.3, how about it?
blocking-b2g: - → 1.3?
Flags: needinfo?(khu)
Reporter | ||
Comment 13•11 years ago
|
||
This is getting really confusing: 1.3 works just fine (see comment 6), as it was 1.1, it's only 1.2 which has this issue. What we're asking is to find the patch that fixed 1.3 and port it to 1.2, since it seems the easiest way out.
P.S. I asked to define "minor", not "minus" ;-)
Reporter | ||
Comment 14•11 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #11)
> Could you please define "minor"? It practically impacts every single
> language except for en-US and languages shorter than en-US (e.g. Chinese).
Forget that, now that I remember even English itself is failing.
Reporter | ||
Updated•11 years ago
|
Attachment #828536 -
Attachment description: 2013-11-07-10-17-45.png → English string cut in settings
Comment 15•11 years ago
|
||
I agree with what Chris proposes in comment 9. Tim, it seems this will help many truncation issues, do you think there's a possibility that this can make it into 1.2? Or will it have to wait for 1.3? Thanks!
Flags: needinfo?(epang) → needinfo?(timdream)
Comment 16•11 years ago
|
||
Hi all,
I am the poster of the email to the Mozilla l10n list: https://groups.google.com/forum/#!topic/mozilla.dev.l10n/mBVbvgzfEIg
I am seeing some comments that suggest this issue would be only fixed in 1.3 (and not in 1.2).
Let me chime in with some thoughts about this issue:
- It impacts the system *globally*
- It is a huge regression from 1.1
- It affects *all* locales (including EN!)
- It is (apparently, per comments) solved in 1.3, so there is already a fix in place
- It renders TVT (translation verification test) for 1.1 useless (i.e. it has to be redone from scratch)
- It is the cause of many truncation bugs that would otherwise not happen (=waste of time for testers)
- It forces locales to shorten the strings (even more) until 1.3, in which strings can be longer again (what is the sense of that?)
If this is not a blocker, I do not know what it is.
The waste of time caused by this issue is enormous for the different locale teams and even for testers who are opening bugs. Théo (comment 6) stresses how massively French local was hit by this. As does Francesco (Italian locale I assume), and I agree that this issue is not minor at all. In fact, I would raise the priority right away.
For Catalan locale (and I guess this can be extrapolated to all locales) in 1.1 we struggled to adapt strings to the allotted space. For some of them, it took several tries (=several builds) to find the shortened version that was "less traumatic". Now, are we supposed to repeat the effort to further shorten strings which are not really needed to be shortened because in 1.3 they would fit? This does not make sense to me.
I stopped testing 1.2 when I realized of this issue and I would advise other locales to wait until this is resolved. Which means fixed, not deferred to 1.3. The overhead caused by this issue is too much. Multiply the efforts by the number of locales.
All this makes no sense from an l10n point of view.
I vote for blocker for 1.2.
Comment 17•11 years ago
|
||
(In reply to Eric Pang [:epang] from comment #15)
> I agree with what Chris proposes in comment 9. Tim, it seems this will help
> many truncation issues, do you think there's a possibility that this can
> make it into 1.2? Or will it have to wait for 1.3? Thanks!
I have no particular opinion on this issue.
Let's feed this bug into 1.2? triage meetings.
blocking-b2g: 1.3? → koi?
Flags: needinfo?(timdream)
Comment 18•11 years ago
|
||
This really shouldn't need any more triage or analysis. The choice is clear.
Restore, or improve, the padding space provided on screens in 1.1 and before, or else we will need to fix several hundred string truncation regressions that have been introduced by recent changes. All this revision of strings across all languages will be wasted work since the bug has been fixed in 1.3.
Comment 19•11 years ago
|
||
Agreed. I don't understand how you could spend so much time thinking about tracking when there is no patch to make.
Anyway, I think the commit we are looking for is https://github.com/mozilla-b2g/gaia/commit/523eb19201562957393068709f0deedcf870928f . Can anyone confirm? I will try to revert it on 1.3
Depends on: 913015
Comment 20•11 years ago
|
||
Please please please can we get the fix quickly on 1.2?
We can't delay our localization test run any longer and we really need this to be fixed in order to start it.
thanks!!!
Comment 21•11 years ago
|
||
Theo or Chris, should anything be set to review? for someone? Are we past that point? What else can happen to move this along? Let me know if I can help.
Comment 22•11 years ago
|
||
(In reply to Théo Chevalier [:tchevalier] from comment #19)
> Agreed. I don't understand how you could spend so much time thinking about
> tracking when there is no patch to make.
>
> Anyway, I think the commit we are looking for is
> https://github.com/mozilla-b2g/gaia/commit/
> 523eb19201562957393068709f0deedcf870928f . Can anyone confirm? I will try to
> revert it on 1.3
This is not the right changeset, sorry.
(In reply to Stephany Wilkes from comment #21)
> Theo or Chris, should anything be set to review? for someone? Are we past
> that point? What else can happen to move this along? Let me know if I can
> help.
We still need to figure out which changeset fixed that on master.
We need to ask someone. Kaze?
No longer depends on: 913015
Updated•11 years ago
|
Flags: needinfo?(kaze)
Assignee | ||
Comment 23•11 years ago
|
||
(In reply to Théo Chevalier [:tchevalier] from comment #22)
> We still need to figure out which changeset fixed that on master.
> We need to ask someone. Kaze?
Pinging my friend Pavel because he might have written the patch that fixed this issue.
Meanwhile, if anyone can narrow the regression window it’d be greatly appreciated.
Flags: needinfo?(kaze) → needinfo?(pivanov)
Keywords: regressionwindow-wanted
Updated•11 years ago
|
Keywords: regression
Comment 24•11 years ago
|
||
It has been fixed between 1.2 merge day (around 15 Sept.) and 8 October
(I don't have builds before 8 October)
Comment 25•11 years ago
|
||
Hey guys,
I think that the right one is Bug 917704 and I think it's save to uplift only this one. Hope this help.
Flags: needinfo?(pivanov)
Assignee | ||
Comment 26•11 years ago
|
||
Théo, Pavel, you rock. Thank you guys. :-)
This bug is about the 1.2 branch and fixing it would require to uplift the patches related to bug 917704 and bug 929565, which didn’t get the approval for 1.2 — see bug 917704 comment 12.
(In reply to chris hofmann from comment #18)
> This really shouldn't need any more triage or analysis. The choice is
> clear.
>
> Restore, or improve, the padding space provided on screens in 1.1 and
> before, or else we will need to fix several hundred string truncation
> regressions that have been introduced by recent changes. All this revision
> of strings across all languages will be wasted work since the bug has been
> fixed in 1.3.
I agree with Chris and I’d really like to solve this issue.
Jason, can we reconsider uplifting these two patches? Would that help if we propose a single, specific patch for the 1.2 branch?
status-b2g-v1.2:
--- → affected
Flags: needinfo?(jsmith)
Comment 27•11 years ago
|
||
To clarify - Does the combination of bug 917704 & bug 929565 automatically fix this bug?
At this point, we can't take approvals at this point, so we probably can't take the patches on bug 917704 & bug 929565 directly. What we're probably going to do here is build a branch-specific patch for 1.2 to fix this bug.
Flags: needinfo?(jsmith)
Assignee | ||
Comment 28•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #27)
> To clarify - Does the combination of bug 917704 & bug 929565 automatically
> fix this bug?
Yes.
> At this point, we can't take approvals at this point, so we probably can't
> take the patches on bug 917704 & bug 929565 directly. What we're probably
> going to do here is build a branch-specific patch for 1.2 to fix this bug.
Working on this right now.
Assignee | ||
Comment 29•11 years ago
|
||
Assignee: nobody → kaze
Status: NEW → ASSIGNED
Assignee | ||
Comment 30•11 years ago
|
||
Assignee | ||
Comment 31•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #829372 -
Flags: review?(arthur.chen)
Attachment #829372 -
Flags: feedback?(pivanov)
Assignee | ||
Comment 32•11 years ago
|
||
Comment on attachment 829372 [details]
link to pull request (1.2 branch)
Théo, would you have a few minutes to test this patch please?
Attachment #829372 -
Flags: feedback?(contact)
Comment 33•11 years ago
|
||
Sounds like we've got a good picture on how to fix this bug at this point, so I'm removing the regression window wanted keyword.
Keywords: regressionwindow-wanted
Comment 34•11 years ago
|
||
koi+ -- this looks like a high impact regression and it's already working on 1.3. It would be odd to let this break for 1.2.
blocking-b2g: koi? → koi+
Comment 35•11 years ago
|
||
Comment on attachment 829372 [details]
link to pull request (1.2 branch)
Flashed a Keon with 1.2 20131104224010, Gaia commit e46434fe0a37d4ceb6dc4172c0a59d06b3f699a8
+ your PR applied on top.
Working as expected, no "Show password" checkbox regression, labels fits like on 1.3 with the same padding. Thanks Kaze!
Attachment #829372 -
Flags: feedback?(contact) → feedback+
Comment 36•11 years ago
|
||
Everything looks good :)
Comment 37•11 years ago
|
||
This is fantastic - thanks guys! Huge improvement.
Comment 38•11 years ago
|
||
Comment on attachment 829372 [details]
link to pull request (1.2 branch)
Works fine :)
Attachment #829372 -
Flags: feedback?(pivanov) → feedback+
Comment 39•11 years ago
|
||
Comment on attachment 829372 [details]
link to pull request (1.2 branch)
r=me. Thanks!
Attachment #829372 -
Flags: review?(arthur.chen) → review+
Assignee | ||
Comment 40•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Comment 41•11 years ago
|
||
Kaze - Does this need to be fixed on master as well? Or is this 1.2 specific?
Flags: needinfo?(kaze)
Assignee | ||
Comment 42•11 years ago
|
||
This is 1.2 specific, as you requested in comment 27. ;-)
Everything works fine on master.
Flags: needinfo?(kaze)
Comment 43•11 years ago
|
||
Just wanted to note here that we're still filing quite a lot of truncation bugs even with this fix: https://bugzilla.mozilla.org/show_bug.cgi?id=928174
17 truncation bugs have been filed yesterday.
Comment 44•11 years ago
|
||
(In reply to delphine from comment #43)
> Just wanted to note here that we're still filing quite a lot of truncation
> bugs even with this fix: https://bugzilla.mozilla.org/show_bug.cgi?id=928174
> 17 truncation bugs have been filed yesterday.
Bug #908309 could probably resolve these bugs.
Comment 45•11 years ago
|
||
Yeah but Bug #908309 won't land in 1.2, and it's not even a blocker for 1.3. Which means that we'll just have to be fixing case by case hundreds of bugs again? I think localizers are more than fed up with us asking them to shorten strings, and asking a case by case UX fix is not the solution
blocking-b2g: koi+ → koi?
status-b2g-v1.2:
fixed → ---
Comment 46•11 years ago
|
||
Oops, didn't mean to touch the flags, what happened?
Please koi+ again :/
status-b2g-v1.2:
--- → fixed
Updated•11 years ago
|
blocking-b2g: koi? → koi+
Comment 47•11 years ago
|
||
(In reply to delphine from comment #45)
> Yeah but Bug #908309 won't land in 1.2, and it's not even a blocker for 1.3.
> Which means that we'll just have to be fixing case by case hundreds of bugs
> again? I think localizers are more than fed up with us asking them to
> shorten strings, and asking a case by case UX fix is not the solution
This is exactly why I point it out! With so many new locales and more to come in the next versinos, I think this bug should be blocker for v1.2, but this is not a decision I can make.
Comment 48•11 years ago
|
||
This bug is a blocker for 1.2. Jason has marked it koi+.
You need to log in
before you can comment on or make changes to this bug.
Description
•