519413 - Text beside icons doesn't work in address book and compose windows.

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect, P4)

Description

[Mark Banner (:standard8)]

For some reason the new text beside icons introduced in bug 509044 doesn't work in the address book and compose windows.

I'm not sure if we want it default there - consistency says possibly, but users should be able to switch it at least as the option is there.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

There is a blocker here, whether it's "make it work" or "make it impossible to select for toolbars where it doesn't".

Comment 2

[Bryan Clark (DevTools PM) [:clarkbw]]

I think we want to address the problem in that order.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: Windows.

Here's what I've got for Windows.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Attachment: Mac OS X.

And the Mac.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Attachment: Ubuntu.

And Gnome.

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Attachment: The patch to make it work.

It's untested on Windows 7/Vista, because the files are still copying, so I'll do that tomorrow.  (Or Andreas could, if he's feeling up for it.)

I've also rolled a fix for bug 519546 into it, because I'm adding a whole lot of places that would suffer from the same problem.

Thanks,
Blake.

Comment 7

[Bryan Clark (DevTools PM) [:clarkbw]]

Looks good in the screenshots I'll try it out on my Vista machine later

Comment 8

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 403678 [details] [diff] [review]
The patch to make it work.

works for me.  now we just need to get the defaults done. :)

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 403678 [details] [diff] [review]
The patch to make it work.

This must be that simple CSS patch you were going to ask me to review last Wednesday.

Comment 10

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/218a1cdcdd31

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Attachment: A silghtly different patch, which is a little too late.

(In reply to comment #9)
> (From update of attachment 403678 [details] [diff] [review])
> This must be that simple CSS patch you were going to ask me to review last
> Wednesday.

Ironically, not quite.

It was going to be, but then I noticed that dmose used 2px of padding in his patch to bug 465138, and then I thought that I might want to only add the padding if we had toolbox[labelalign="end"] and toolbar[mode="full"], so that we didn't add the padding when the user switched from "Icons beside Text" to text.

(You can see this by customizing the toolbar, switching to "Icons and Text", switching to "Text", making note of the position, then switching to "Icons beside Text", and switching back to "Text", and seeing how things are slightly different.)

Is it worth me updating this patch to handle that case?  My gut feeling is that I should probably spend my time on making the autoconfig code more robust, but if you feel otherwise, I'ld be happy to spend some more time on this.

Thanks,
Blake.