1334814 - Some Preferences panels components cut off on the right side

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Javi Rueda :javirid]

Attachment: Screenshot of one of the affected panes

Testing latest Thunderbird 52b1, I opened Preferences. Thunderbird locale was Catalan (ca-ES). Operating system was es-ES.

The same is happening in Calendar and Advanced tabs. The other ones are ok.

No messages in the Error Console are appearing.

Build ID: 20170126105536

Comment 1

[Javi Rueda :javirid]

Thunderbird 51.b2 only shows this problem in Advanced tab. General tab was ok.

Comment 2

[patclash]

Have you try to display the Preferences in a tab ? 

open about:config, search "mail.preferences.inContent" and set it to "true" (default is "false")

Comment 3

[Javi Rueda :javirid]

Tried with preferences in-content. Same issue.

Comment 4

[Wayne Mery (:wsmwk)]

did it happen in older version, including 45.x?
if it did not, what is last known version to not be a problem?

Comment 5

[Richard Marti (:Paenglab)]

Attachment: bug1334814.patch

The General pane is too wide because the description doesn't make a line break because it is in a <hbox> -> removed the <hbox>.

The Advanced pane issue is no new change and is already in TB 45. This can't be fixed by by me. This needs a change in the locale. http://hg.mozilla.org/l10n-central/ca/file/tip/mail/chrome/messenger/preferences/preferences.dtd#l9 needs to be changed from 52em to 58em.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: Screenshot of the General pane with the patch

Comment 7

[Javi Rueda :javirid]

Good. Thank you, Richard :)

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 8831530 [details] [diff] [review]
bug1334814.patch

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

This stuff is almost 100% unreadable. Tags are opened outside and closed inside a conditional cause. And inside the conditional clause more tags are opened and then closed outside the conditional clause.

While removing one such case in the patch, there is room for another improvement here. All tags should always be opened and closed inside the same branch of an if clause. Had that been the case, this would have been a two minute review, in the current state I had to look at the original file to see that the patch is right.

::: mail/components/preferences/general.xul
@@ -70,2 @@
>        <caption label="&newMessagesArrive.label;"/>
>        <hbox align="center">

Please move the opening tag into the if clause and duplicate it also into the else part.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: bug1334814.patch v2

Is this better?

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: bug1334814.patch (v3)

Better, but I can make it perfect ;-)
Can you please try it to make sure I haven't broken anything.

Comment 11

[Jorg K (CEST = GMT+2)]

Sorry, you wanted minimal change and I'm promoting a full-blown clean-up here: Move the end tag where it belongs and fix the indentation while we're here.

Comment 12

[Richard Marti (:Paenglab)]

Comment on attachment 8831545 [details] [diff] [review]
bug1334814.patch (v3)

Still working. :-)

Comment 13

[Jorg K (CEST = GMT+2)]

You could land this since there's been an M-C merge at about 18:00h.

Comment 14

[Richard Marti (:Paenglab)]

You're right:-)

https://hg.mozilla.org/comm-central/rev/07ccae5d880ba9f0e798b061abfe00fb100568d0

Comment 15

[Richard Marti (:Paenglab)]

Comment on attachment 8831545 [details] [diff] [review]
bug1334814.patch (v3)

[Approval Request Comment]
User impact if declined: too wide panel on some locales
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 16

[Jorg K (CEST = GMT+2)]

Comment on attachment 8831545 [details] [diff] [review]
bug1334814.patch (v3)

OK ;-)

Comment 17

[Jorg K (CEST = GMT+2)]

Who's fixing
http://hg.mozilla.org/l10n-central/ca/file/tip/mail/chrome/messenger/preferences/preferences.dtd#l9

¡Visca el Barça i visca Catalunya! ;-)

Comment 18

[Javi Rueda :javirid]

(In reply to Jorg K (GMT+1) from comment #17)
> Who's fixing
> http://hg.mozilla.org/l10n-central/ca/file/tip/mail/chrome/messenger/
> preferences/preferences.dtd#l9
> 
> ¡Visca el Barça i visca Catalunya! ;-)

I know that it is not a good behavior here in Bugzilla to do what I am going to do, but: THANK YOU, Jorg!

Comment 19

[Jorg K (CEST = GMT+2)]

(In reply to Javi Rueda from comment #18)
> ... not a good behavior here in Bugzilla ...
¿Què té de dolent? Sorry, I don't really speak Català (only Spanish). Everybody loves football and Catalunya ;-)
And I avoided a political discussion by not adding the magic word "lliure" ;-)
Anyway, you have to thank Richard for the patch. This will be uplifted and you'll have it in TB 52.

Now back to my question: Who's fixing the l10n-central/ca file?

Comment 20

[Javi Rueda :javirid]

(In reply to Jorg K (GMT+1) from comment #19)
> (In reply to Javi Rueda from comment #18)
> > ... not a good behavior here in Bugzilla ...
> ¿Què té de dolent? Sorry, I don't really speak Català (only Spanish).

Res (Anything). I think that it is a bad practice to talk here about what is unrelated to any bug. I read it in another bug for Firefox.

> Anyway, you have to thank Richard for the patch. This will be uplifted and
> you'll have it in TB 52.

I did it, but I can do it again. Thank you, Richard for your fast fix of this problem! :)

> 
> Now back to my question: Who's fixing the l10n-central/ca file?

I did a search in Bugzilla and didn't found any bug report. There was, however one for the same problem for Firefox, also in the Advanced pane, this time in Windows (https://bugzilla.mozilla.org/show_bug.cgi?id=394791).

I didn't report a new one because Richard said in comment 5 he could fix that.

If he is unable to, I could, but I have never done any fix for that repo.

Comment 21

[Jorg K (CEST = GMT+2)]

(In reply to Javi Rueda from comment #20)
> I didn't report a new one because Richard said in comment 5 he could fix
> that.
He said that he could NOT fix that: "This can't be fixed by me." == "I can't fix that."

> If he is unable to, I could, but I have never done any fix for that repo.
You'd have to talk to the localisers who control that repository.

Comment 22

[Richard Marti (:Paenglab)]

Thank you, Javi for filing this bug. :-)

I wrote, I can't fix it. You don't need to fix the bug but file one on l10n-central/ca. Then the localizers can fix it.

Comment 23

[Jorg K (CEST = GMT+2)]

Aurora (TB 53):
https://hg.mozilla.org/releases/comm-aurora/rev/d1ad7b83b1fea4570710b9efbff8fa6397b40a5f

Comment 24

[Jorg K (CEST = GMT+2)]

Beta (TB 52):
https://hg.mozilla.org/releases/comm-beta/rev/39fe7977c37c659d5656787b49c4812d66072f6b

Comment 25

[Javi Rueda :javirid]

(In reply to Richard Marti (:Paenglab) from comment #22)
> Thank you, Javi for filing this bug. :-)
> 
> I wrote, I can't fix it. You don't need to fix the bug but file one on
> l10n-central/ca. Then the localizers can fix it.

Ok. I misunderstand that. Thank you for fixing this for Beta and Aurora, Richard.

I have just filed bug 1336818 about replacing the width from 52 to 58em.