Closed Bug 996159 Opened 10 years ago Closed 10 years ago

Sync panel is still not entirely displayed in some localized builds

Categories

(Firefox :: Settings UI, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 + verified
firefox30 + verified
firefox31 + verified

People

(Reporter: wladow, Assigned: ttaubert)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #982615 +++

Disconnect button is still cut on Windows 7 and 8.1 in the Slovak build (and possibly others), probably repros on all platforms.

I have managed to workaround this by shortening the
      signedInLoginFailure.beforename.label
in the following block of
      http://hg.mozilla.org/mozilla-central/file/7b6905940e9a/browser/components/preferences/sync.xul

              <hbox flex="1">
                 <description>
                   &signedInLoginFailure.beforename.label;
                   <span id="fxaEmailAddress3"></span>
                   &signedInLoginFailure.aftername.label;
                 </description>
                ...
              </hbox>

Since it's a workaround let's try to fix the root cause here instead.

As per bug 982615 comment 20 we may need to revisit also the following:
               <!-- logged in to an unverified account -->
              <hbox flex="1">
                <description>
                  &signedInUnverified.beforename.label;
                  <span id="fxaEmailAddress2"></span>
                  &signedInUnverified.aftername.label;
                </description>
                ...
              </hbox>
The issue can be reproduced with German and French (!) on Windows 8.1 (with 125% dpi on OS level) which is in contrary to bug 982615 comment 21.
I can reproduce this by changing the English strings to longer values.  I had a quick look and couldn't find the secret sauce, and I'm not going to be able to look again for 4-5 days.

Tim - you seem to understand this stuff well - any ideas?
Flags: needinfo?(ttaubert)
Attached patch patch, v1 (obsolete) — Splinter Review
Successful Try run: https://tbpl.mozilla.org/?tree=Try&rev=7a16ff6795e7
Assignee: nobody → archaeopteryx
Status: NEW → ASSIGNED
Attachment #8408055 - Flags: review?(ttaubert)
Flags: needinfo?(ttaubert)
It would be great if we could have a patch in m-b for today 13 PDT... To be sure it makes it in beta 9 (gtb today).
Comment on attachment 8408055 [details] [diff] [review]
patch, v1

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

Thanks for working on this, Sebastian. This patch unfortunately doesn't fix the problem on my Win 7 VM, with German strings and 125% DPI. I dug a little deeper and I think we have a problem with <spacer> elements at non-default Windows DPI settings. As soon as the email addresses are injected the layout somehow breaks. The only thing I could find that sounds related is bug 886198.

So my suggestion is to remove all <spacer> elements in the #fxaLoginStatus deck and instead assign flex="1" to the spacers' predecessors. I did a quick test and that seems to work fine for me.
Attachment #8408055 - Flags: review?(ttaubert) → review-
Taking over from here. Sebastian unfortunately had a few problems reproducing the issue and I offered to upload the patch I almost completed when reviewing.

Sebastian, does this work for you? Do you still see the issue?
Assignee: archaeopteryx → ttaubert
Attachment #8408055 - Attachment is obsolete: true
Attachment #8408279 - Flags: feedback?(archaeopteryx)
Comment on attachment 8408279 [details] [diff] [review]
0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

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

The views 'pending verification' and 're-auth because login failed' wrap fine. The 'logged in and all fine' view doesn't wrap (because no hyphenation) and can push the 'Manage' link and 'Disconnect' button off the screen. The limit for this is ~65 latin characters. I'd suggest to land this patch and fix the last with cropping and showing the whole mail address as tooltip (because hyphenation makes mail address ambiguous).
Attachment #8408279 - Flags: feedback?(archaeopteryx) → feedback+
Tim, can we get an uplift request? thanks
Flags: needinfo?(ttaubert)
Comment on attachment 8408279 [details] [diff] [review]
0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

As mentioned by Sebastian, we still break for really long email addresses but this doesn't seem to be new :/ We however can unbreak people on 125% DPI Windows Systems.

Gavin, can you review this given that Mark is out for a few days?
Attachment #8408279 - Flags: review?(gavin.sharp)
Flags: needinfo?(ttaubert)
The long email issue is tracked in bug 991691, FWIW.
Comment on attachment 8408279 [details] [diff] [review]
0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

Does this mess up the focus outlines for the links (i.e. regress bug 966124)? I guess not since it's just the parent vbox being flexed.
Attachment #8408279 - Flags: review?(gavin.sharp) → review+
Flags: firefox-backlog?
Comment on attachment 8408279 [details] [diff] [review]
0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

I am forcing the uplift to beta. We do want this in beta 9 (gtb very soon).
Attachment #8408279 - Flags: approval-mozilla-beta+
Comment on attachment 8408279 [details] [diff] [review]
0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

idem for aurora
Attachment #8408279 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Verified that the Disconnect button appears correctly on Firefox 29 beta 9 SK build. Tested on Win 7, 64-bit (reproduced for normal and higher res - now fixed), Ubuntu 32-bit (SK and DE build), Mac OS X 10.8.5.
https://hg.mozilla.org/mozilla-central/rev/7af4241ce919
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Flags: firefox-backlog?
Verified as fixed on Firefox 30 beta 3 (20140508121358) and latest Aurora 31.0a2 (20140512004006) under Win 7 64-bit, Ubuntu 12.10 32-bit and Mac OSX 10.8.5.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.