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)

defect
Not set
normal

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: flod, Assigned: kaze)

References

Details

(Keywords: regression)

Attachments

(6 files)

Attached image Comparison between 1.1 and 1.2 (deleted) —
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 ;-)
asking for UX help about this. thanks
Flags: needinfo?(firefoxos-ux-bugzilla)
Flagging Eric to save the day again. :)
Flags: needinfo?(epang)
Flags: needinfo?(firefoxos-ux-bugzilla)
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)
Who is the right person to create the patch that adds the padding? Lets use this bug to track that work.
blocking-b2g: --- → leo?
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?
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
ni? Eric Pang as he seems to have worked on this issue earlier
Flags: needinfo?(epang)
(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.
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
Taipei triage result = minus it, because it looks like a minor issue.
blocking-b2g: koi? → -
(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)
(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)
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" ;-)
Attached image English string cut in settings (deleted) —
(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.
Attachment #828536 - Attachment description: 2013-11-07-10-17-45.png → English string cut in settings
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)
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.
(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)
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.
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
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!!!
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.
(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
Flags: needinfo?(kaze)
(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: regression
It has been fixed between 1.2 merge day (around 15 Sept.) and 8 October (I don't have builds before 8 October)
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)
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?
Flags: needinfo?(jsmith)
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)
(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.
Attached file link to pull request (1.2 branch) (deleted) —
Assignee: nobody → kaze
Status: NEW → ASSIGNED
Attached image screenshot — before patch (deleted) —
Attached image screenshot — after patch (deleted) —
Attachment #829372 - Flags: review?(arthur.chen)
Attachment #829372 - Flags: feedback?(pivanov)
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)
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.
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 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+
Attached image 1.2 with patch applied (deleted) —
Everything looks good :)
This is fantastic - thanks guys! Huge improvement.
Comment on attachment 829372 [details] link to pull request (1.2 branch) Works fine :)
Attachment #829372 - Flags: feedback?(pivanov) → feedback+
Comment on attachment 829372 [details] link to pull request (1.2 branch) r=me. Thanks!
Attachment #829372 - Flags: review?(arthur.chen) → review+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Kaze - Does this need to be fixed on master as well? Or is this 1.2 specific?
Flags: needinfo?(kaze)
This is 1.2 specific, as you requested in comment 27. ;-) Everything works fine on master.
Flags: needinfo?(kaze)
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.
(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.
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?
Oops, didn't mean to touch the flags, what happened? Please koi+ again :/
blocking-b2g: koi? → koi+
(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.
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.

Attachment

General

Created:
Updated:
Size: