996159 - Sync panel is still not entirely displayed in some localized builds

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Vlado Valastiak [:wladow]]

Attachment: Slovak Central Win7 build 20140413030203

+++ 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>

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

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?

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v1

Successful Try run: https://tbpl.mozilla.org/?tree=Try&rev=7a16ff6795e7

Comment 4

[Sylvestre Ledru [:Sylvestre]]

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 5

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

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?

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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).

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Tim, can we get an uplift request? thanks

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

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?

Comment 10

[:Gavin Sharp [email: [e-mail]]]

The long email issue is tracked in bug 991691, FWIW.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

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).

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8408279 [details] [diff] [review]
0001-Bug-996159-Fix-sync-config-section-on-Windows-with-1.patch

idem for aurora

Comment 14

[Sylvestre Ledru [:Sylvestre]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bd6fdc0dcac0

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/7af4241ce919
https://hg.mozilla.org/releases/mozilla-aurora/rev/e43434b6dd4e

Comment 16

[Petruta Horea [:phorea], Desktop QA]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7af4241ce919

Comment 18

[Petruta Horea [:phorea], Desktop QA]

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.