Sync has several layout issues on localized builds (I'm using the Italian build, but I'm quite sure that other languages have similar problems). I posted a comment in bug 591122 four months ago but nothing has changed and I don't really want to see a final release shipping with this kind of problems. Build ID: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20110101 Firefox/4.0b9pre This problems involve the add-on (ver. 1.6.1) as well.

Three different issues: (1) (not l10n specific) strange sign when you select the checkbox (2) unwanted space between two strings (that space is somewhere in the layout, not in the strings, and that's an error in Italian). (3) unwanted horizontal scroolbar

Sorry, my previous comment is obviously quite confused. 1) unwanted horizontal scrollbar 2) (minor, not l10n specific) sign on the checkbox 3) unwanted space between two strings The screenshot was taken on Windows 7 but the same happens on Mac OS X

This window is displayed when you try to change the Sync Key. Strings are completely cut, half a button is missing.

Last note: I'm aware that the right place for this bug should be Mozilla Services->Firefox Sync: UI, but I think that this kind of problem should be blocking the final release of Firefox 4.

Philipp?

FWIW, we also have this problem in the new account window in French. I'm pretty sure it is caused by the checkbox label (which doesn't wrap), since if you choose to use a custom server the problem disappears. We also have this problem in Change Sync Key, just like in comment #3. I can also see an horizontal scrollbar in the reset sync dialog (three radio buttons). I agree this should be a blocker for Firefox 4 final.

Benoit is absolutely right, I missed that one.

Philipp, care to explain why do you think that this kind of problem shouldn't be a blocker (I suppose so, since you moved the bug to services)? If that's not the case, could you at least set a dependency to bug 591122 and set that bug as a blocker?

(In reply to comment #8) > Philipp, care to explain why do you think that this kind of problem shouldn't > be a blocker (I suppose so, since you moved the bug to services)? I definitely think this should be a blocker. Moving it to services has no impact on that. Sync UI bugs are just tracked here and not in the Firefox component. > If that's not the case, could you at least set a dependency to bug 591122 and > set that bug as a blocker? You're right, it should block bug 591122. Not sure if that itself is a blocker (I don't think we need to hold the release for it.)

The main problem here seems to be that the checkbox labels do not wrap, and the Italian strings are nearly twice as long as the English ones. Looking at this now, though given my inexperience with XUL I can't make any promises!

(In reply to comment #10) > The main problem here seems to be that the checkbox labels do not wrap, and the > Italian strings are nearly twice as long as the English ones. We're not even using a label for the checkbox (which is kind of a bug on its own, I think it has been filed even), we're using a <decription/> which *should* wrap, but the links inside that description are <label/>s and those don't wrap.

Actually I think <label>foo</label> wraps but <label value="foo"/> doesn't.

Re item 3 in Comment 2: removing this space would require a string change. The labels are joined with spaces, so to get the "l'" to touch "informativa" either: * Every localization that doesn't use an elided article here would need to be amended to introduce spacing, and the XUL altered to remove the implicit space. * Every localization that does use an elided article here would need to move the article (e.g., "l'") from setup.tosAgree2.label into setup.tosAgree3.label. If you want to try the second, go right ahead; it's a l10n change, not a code change.

(In reply to comment #13) > * Every localization that does use an elided article here would need to move > the article (e.g., "l'") from setup.tosAgree2.label into setup.tosAgree3.label. > > If you want to try the second, go right ahead; it's a l10n change, not a code > change. Yes, this is what we should do. See bug 575601 which is precisely about this issue.

Here's a change to the Sync setup dialog to flex/wrap those labels. Tested by putting excessively long strings in the DTD (thanks Philipp!).

Adding a dependency on Bug 575601 -- the whitespace issue will either be fixed there, or WONTFIXed, but either way it won't happen in this bug.

Comment on attachment 503672 [details] [diff] [review] XUL change for label wrapping. v1 > &setup.tosAgree2.label; > <label class="text-link inline-link" >- onclick="event.stopPropagation();gSyncUtils.openPrivacyPolicy();" >- value="&setup.ppLink.label;"/> >- &setup.tosAgree3.label; >+ onclick="event.stopPropagation();gSyncUtils.openPrivacyPolicy();"> >+ &setup.ppLink.label; >+ &setup.tosAgree3.label; >+ </label> &setup.tosAgree3.label; should be outside the </label> like before since it's not part of the link. > <label id="wipeRemoteLabel" >- control="wipeRemote" >- value="&choice2.server.main.label;"/> >+ control="wipeRemote"> >+ &choice2.server.main.label; >+ </label> Nit: fix indention for control="" please. This should be it for this bug, I'm addressing the layout issues for the Sync Key dialog in bug 619013.

Pushed http://hg.mozilla.org/mozilla-central/rev/01ef10d604ab and of course forgot to update patch according to my review comments, so I also pushed http://hg.mozilla.org/mozilla-central/rev/97e26cd7f945

(In reply to comment #13) > Re item 3 in Comment 2: removing this space would require a string change. The > labels are joined with spaces, so to get the "l'" to touch "informativa" > either: Now I understand Philipp's comment in the other bug (sorry but I saw and answered that comment before this one). You're adding content outside our (localizer) scope/control, and that's wrong. If you need spaces between strings those should be in the entities, not in the code (this is the way it always worked). I can fix it using your suggestion, but this doesn't change the fact that this is not the proper way to work with i18n.

(In reply to comment #19) > I can fix it using your suggestion, but this doesn't change the fact that this > is not the proper way to work with i18n. Ok, understood! That's why I didn't close bug 575601 ;). We should fix this then, eventually, but I'm afraid we have to focus on the blockers for now.