Last Comment Bug 713919 - Toolbar buttons dragged onto the stand-alone window menubar should have text-beside-icon
: Toolbar buttons dragged onto the stand-alone window menubar should have text-...
Status: RESOLVED FIXED
: regression
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 12.0
Assigned To: Mike Conley (:mconley) - (Needinfo me!)
:
Mentors:
Depends on:
Blocks: tb-tabsontop
  Show dependency treegraph
 
Reported: 2011-12-28 12:28 PST by Mike Conley (:mconley) - (Needinfo me!)
Modified: 2012-01-31 10:33 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
The text is below the icon, not beside like it should be. (22.90 KB, image/png)
2011-12-28 12:28 PST, Mike Conley (:mconley) - (Needinfo me!)
no flags Details
Patch v1 (1.98 KB, patch)
2011-12-29 11:20 PST, Mike Conley (:mconley) - (Needinfo me!)
sid.bugzilla: review+
bwinton: ui‑review+
standard8: approval‑comm‑aurora+
Details | Diff | Review

Description Mike Conley (:mconley) - (Needinfo me!) 2011-12-28 12:28:36 PST
Created attachment 584617 [details]
The text is below the icon, not beside like it should be.

Steps to reproduce:

1)  Open up a stand-alone message window
2)  Customize the mail toolbar
3)  Drag a toolbar button onto the menubar

What happens?

The text for the toolbar button is below the icon.

What's expected?

The text should be beside the toolbar button.

See the attached screenshot for a look at the bug in action.
Comment 1 Mike Conley (:mconley) - (Needinfo me!) 2011-12-29 11:20:19 PST
Created attachment 584801 [details] [diff] [review]
Patch v1

This patch makes it so that the navigation-toolbox defaults to icons-beside-text in every place it is displayed, as opposed to just messenger.xul.
Comment 2 Mike Conley (:mconley) - (Needinfo me!) 2011-12-30 12:39:12 PST
Comment on attachment 584801 [details] [diff] [review]
Patch v1

Cancelling review - this patch overrides the user preference to display icons above text, or icons only, and we don't want to do that.
Comment 3 Mike Conley (:mconley) - (Needinfo me!) 2012-01-06 13:52:58 PST
Hm - so it turns out this is harder than it sounds, because the customization code assumes we have a single toolbox as opposed to a toolbox with external toolbars that might exist within other toolboxes...
Comment 4 Mike Conley (:mconley) - (Needinfo me!) 2012-01-09 07:18:21 PST
Comment on attachment 584801 [details] [diff] [review]
Patch v1

So here's what I've found out:

The customize toolbar functions in mailCore.js do not take into account the possibility that toolbars can exist outside of their toolboxes.  This means that, with this patch, switching to "text-below-icon" mode will not be obeyed by buttons in the navigation-toolbox.

The changes required by mailCore.js aren't crazy difficult, but I don't think we'd want to land them in Aurora this late in the game.

I suggest that we default to displaying text beside icons in the menubar, and then file a separate bug and fix the customize toolbar regression in a later version (TB 12, for example) in order to allow proper testing time.

Does this sound alright?
Comment 5 Siddharth Agarwal [:sid0] (inactive) 2012-01-09 14:54:33 PST
Comment on attachment 584801 [details] [diff] [review]
Patch v1

To be honest, I prefer this over text below icons in the menu bar. I'm going to let Blake make the call, though.
Comment 6 Blake Winton (:bwinton) (:☕️) 2012-01-11 13:04:28 PST
Comment on attachment 584801 [details] [diff] [review]
Patch v1

I also prefer it, and I'm not sure if we should force it to match the setting for the toolbars, since that's a very different context.

One thing I think we should definitely communicate is that this is just temporary, because of time pressure, and we may change it later to follow the toolbar setting.
Comment 7 Mike Conley (:mconley) - (Needinfo me!) 2012-01-18 06:49:16 PST
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/ef508aa56358
Comment 8 Mike Conley (:mconley) - (Needinfo me!) 2012-01-31 10:33:00 PST
Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/415d430378aa

Note You need to log in before you can comment on or make changes to this bug.