582801 - Thunderbird tab bar should be a toolbar

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, enhancement)

Description

[Jim Porter (:squib)]

Firefox 4.0 has done this (or will do this, depending on your perspective), and I think it would simplify/fix some of the things going on with the tab bar in Thunderbird these days. Here are some benefits:

* Lightning buttons could be customizable
If you don't like the Lightning buttons where they are now, or if your tab bar is hidden, you'd be able to move them to another toolbar or remove them altogether.

* Other buttons could go where the Lightning buttons are now
This would make things easier for extensions or if, say, you wanted the address book button to go there, as in bug 457270 comment 0.

* The Quickfilter button could be moved/removed
Some people don't like the quick filter, or can't use it well because they don't have tabs. This would help solve part of the problem.

Comment 1

[Jim Porter (:squib)]

asuth, any thoughts on this?

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

I don't really know enough about how the firefox tab implementation is changing, but I definitely know it's a separate issue from being able to move the quick filter bar around or remove it.

In general, if someone can provide a patch that largely reuses the existing Firefox work without just copy-and-pasting it so we can reuse bug fixes too, I could see it being something that could be accepted in core.  (Customizable toolbars and XBL widgets tend to mix in bug-creating ways that have high review burdens and possibly high test matrix burdens, so any amount of new or modified existing code would probably be hard to justify unless the lack of customization suddenly is perceived as a major UX problem.)

Comment 3

[Jim Porter (:squib)]

Sorry, I should clarify. I just mean the button to activate the quickfilter. I recall at least one bug report where someone was confused about how the quickfilter worked because their tab bar was hidden.

I'll try to find the Firefox version of this bug and see how simple it would be to port. It's a shame that the tab browsing stuff in Firefox isn't in the core. (I recall there being a bug for that too, though.)

Comment 4

[Jim Porter (:squib)]

This is partly for my own reference for when I get around to looking at this: in 3.3, we could use external toolbars like Firefox 4 and have the tab-bar toolbar use the same icon set as the main toolbar: https://developer.mozilla.org/en/XUL/toolbar

This would be nice for Lightning, since they already add non-default toolbar buttons to the main toolbar palette, and they could just use those on the hypothetical tab-bar toolbar instead of what they have now.

Comment 5

[Jim Porter (:squib)]

A difficulty: you can't add external toolbars from an XBL binding and have persistence across sessions work right (but it seems like you can if you use an overlay instead). There are a few ways you could get around this, e.g. making tabpanels not be children of the tabmail element and then wrapping tabmail with a toolbar, but they're probably all pretty complicated.

:asuth, any ideas?

(Also, :clarkbw, I CC'ed you since you were looking at address-book-in-a-tab, and this might help with that.)

Comment 6

[Jim Porter (:squib)]

Attachment: WIP patch

This patch lets you modify the right side of the tab bar, where the quick filter button usually goes. I still need to do some style work, and the icon for the qfb needs to be changed so that it's obvious when it's disabled, but it's a start.

Comment 7

[Jonathan Protzenko [:protz]]

Works fine for me, as far as I could tell. This probably will warrant a strong piece of communication to make sure that addon authors now overlay the right toolbar (possibly with a sample chrome.manifest that uses version= clauses). I think this will be a great addition to Thunderbird :)

Comment 8

[Jim Porter (:squib)]

Attachment: Add a tab bar where the tabmail buttons used to be

Here's a finished patch for this. I'm not sure the best way to add the quick filter button as a default button on the tab bar toolbar, so I just set the defaultset attribute. This is a bit strange, since the rest of the QFB code is done in overlays. As far as I know, adding a toolbar button from an overlay requires using Javascript, which seems like overkill here.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 546467 [details] [diff] [review]
Add a tab bar where the tabmail buttons used to be

The quick-filter-bar stuff is just an overlay because it started out life as an extension and it seemed helpful to keep its stuff factored out in its own file.

This needs UX sign-off because:
- I believe the styling of the quick-filter-bar button assumes it will live exactly where it lives.
- The ability to lose an important piece of the UI needs sign-off, noting that there is unlikely to be complicated technical fall-out from the removal of the button other than code that assumes the button is present and dies when it gets a null back, but you appear to have dealt with that.

No unit test is required given the simplicity of the button and the likely isolated nature of any breakage that would occur to bits that are irrelevant when the button is removed anyways.

Comment 10

[Jim Porter (:squib)]

(In reply to comment #9)
> The quick-filter-bar stuff is just an overlay because it started out life as
> an extension and it seemed helpful to keep its stuff factored out in its own
> file.

That's what I figured. I just wanted to make sure. :)

> - I believe the styling of the quick-filter-bar button assumes it will live
> exactly where it lives.

I guess you're referring to the Windows tab-style button? That should be helped by bug 670304 and bug 667244, but I suppose the latter should block this bug.

> - The ability to lose an important piece of the UI needs sign-off, noting
> that there is unlikely to be complicated technical fall-out from the removal
> of the button other than code that assumes the button is present and dies
> when it gets a null back, but you appear to have dealt with that.

I tested it out, and the way it's currently implemented, clicking "restore default set" makes the icon reappear just like the other toolbar buttons, which is good.

Comment 11

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

Comment on attachment 546467 [details] [diff] [review]
Add a tab bar where the tabmail buttons used to be

Well, uh, I like the direction this patch is going, I think…  But…

when I drag the Quick Filter button off of the toolbar, I see the following error:

JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/search.xml ::  :: line 156"  data: no]

And I don't see anything in the customize pane…

Moving things off the toolbar onto the tab bar makes for some weird interaction.

If I move some stuff off, then it starts looking a little wierd:
http://dl.dropbox.com/u/2301433/Toolbar/MovingSome.png

Moving everything off makes it look weirder:
http://dl.dropbox.com/u/2301433/Toolbar/MovingAll.png

(Another problem here is that the dropdown sheet is over the tab bar, but I can place things under it.  If the tab bar is part of the customizable area, I thinkg the dropdown should appear under it, too.)

Finally, I moved everything off the toolbar, and hid it to get:
http://dl.dropbox.com/u/2301433/Toolbar/LessHeader.png
which is kinda pretty, and spectacularly minimal!  ;)

But then I hit Restore Default Set, and re-showed the toolbar, and there was nothing on it!  Aaaah!  (Hitting Restore Default Set again fixed it, but that seemed really bad.)

So, given all that, I think I've got to say ui-r-.  :(

Thanks,
Blake.

Comment 12

[Richard Marti (:Paenglab)]

Attachment: Top and bottom border on tab toolbar

I tried today your patch under Win7. As you can see on the screenshot a top- and bottom border are visible coming from toolbar.css.

It looks you have to add in messenger-aero.css a border-top: none and a border-bottom: none

Comment 13

[Richard Marti (:Paenglab)]

(In reply to Richard Marti [:paenglab] from comment #12)
> Created attachment 550828 [details]
> Top and bottom border on tab toolbar
> 
> I tried today your patch under Win7. As you can see on the screenshot a top-
> and bottom border are visible coming from toolbar.css.
> 
> It looks you have to add in messenger-aero.css a border-top: none and a
> border-bottom: none

The same happens under XP. So it's enough to add the two lines in messenger.css

Comment 14

[Jim Porter (:squib)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #11)
> JavaScript error: , line 0: uncaught exception: [Exception... "Component
> returned failure code: 0x80004005 (NS_ERROR_FAILURE)
> [nsIObserverService.removeObserver]"  nsresult: "0x80004005
> (NS_ERROR_FAILURE)"  location: "JS frame ::
> chrome://messenger/content/search.xml ::  :: line 156"  data: no]

I see this error with an unpatched version of Thunderbird too. I'm not sure what's happening here...

> And I don't see anything in the customize pane…

This should be fixed in my local version.
 
> Moving things off the toolbar onto the tab bar makes for some weird
> interaction.
> 
> If I move some stuff off, then it starts looking a little wierd:
> http://dl.dropbox.com/u/2301433/Toolbar/MovingSome.png

Is this screenshot during customization or after?

> But then I hit Restore Default Set, and re-showed the toolbar, and there was
> nothing on it!  Aaaah!  (Hitting Restore Default Set again fixed it, but
> that seemed really bad.)

This appears to be a Core issue: if a toolbar button is on an external toolbar that's *after* the main toolbar in the DOM, restoring the defaults doesn't work right (try dragging the Home button to the Add-on Bar in Firefox). I'll try to remember to file a bug on it tomorrow.

Comment 15

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

Comment on attachment 546467 [details] [diff] [review]
Add a tab bar where the tabmail buttons used to be

So, here's what I'm going to do.  I'm changing this to ui-r+, and I've filed bug 687836 to fix any follow-up issues, but please don't commit this until that bug is also ready to land, so that we can land them both at the same time.

Thanks,
Blake.

Comment 16

[Ludovic Hirlimann [:Usul]]

both bugs seem ready to land.

Comment 17

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

Not until we've fixed all the issues in bug 687836!

Comment 18

[Mike Conley (:mconley) (:⚙️)]

I believe this has landed in comm-central as http://hg.mozilla.org/comm-central/rev/24a39254d6a8 as part of the amalgamation of bug 687836.  squib, can you confirm?