Closed Bug 343251 Opened 18 years ago Closed 18 years ago

add "all tabs" menu to tabstrip to address usability problems with tab overflow / scrolling

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: brian.utterback, Assigned: moco)

References

(Depends on 1 open bug)

Details

(Keywords: fixed1.8.1, Whiteboard: [mustfix])

Attachments

(21 files, 25 obsolete files)

204.03 KB, image/tiff
Details
77.07 KB, image/tiff
Details
104.35 KB, image/png
Details
77.71 KB, image/png
Details
232.81 KB, image/bmp
Details
103.97 KB, image/bmp
Details
77.51 KB, image/png
Details
23.67 KB, image/png
Details
99.25 KB, image/png
Details
23.83 KB, image/png
Details
191.13 KB, image/png
Details
46.45 KB, image/png
Details
1.88 KB, image/png
Details
1.77 KB, image/png
Details
11.12 KB, text/plain
Details
11.96 KB, image/png
Details
83.43 KB, image/png
Details
23.82 KB, image/png
Details
153.88 KB, image/tiff
Details
8.78 KB, image/png
Details
26.95 KB, patch
moco
: review+
mconnor
: approval1.8.1+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.8.1a3) Gecko/20060629 Firefox/2.0a3
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.8.1a3) Gecko/20060629 Firefox/2.0a3

The problem reported with 221684 was that once you got to 25 tabs, you couldn't navigate to any subsequent tabs. The fix was to make the tabs a fixed width and
add scolling. This makes tabs nearly unusable after 15 tabs or so. There is no way
to tell where you are in the tabs so you cannot tell which way to scroll. New tabs are still added to the end, so it may be very difficult to navigate between the
current tab and newly created tab. In fact, without the feed back provided by the tabs resizing, it becomes difficult to know if you have made a new tab or not.

Reproducible: Always

Steps to Reproduce:
1. Open a lot of tabs.
2. Start using them.
3.
Comments probably should be posted in the bug in question. Invalid?
This issue might be addressed by one of the many bugs depending on or blocking bug 318168. If you feel that there's any aspect missing (which doesn't include reverting the whole patch), please state this in a positive way (i.e. how should it behave instead).

This bug might be a DUPE, it could be INVALID but there could just as well be something worth addressing.
I second this.  I'm happy to see the progress made in attempting to address these issues but feel that the current implementation of Tab Scrolling is cludgy, slow and makes dealing with many, many tabs much harder IMO.

~B
Blocks: tab-overflow
Summary: Fix for 221684 is worse than the disease. → Fix for 221684 (tab overflow) is worse than the disease.
OS: Solaris → All
Hardware: Sun → All
I can now only have 9 tabs visible on a 1280px-wide screen. This is hurting my usability badly. I switched from IE *because* I didn't have to have huge numbers of windows to keep track of my tabs. Now things get lost, things open invisibly, and basically it just is much harder to use the browser. :-(
Setting browser.tabs.tabMinWidth to a small value (such as 20) and restarting Firefox seems to help.
See also bug 344048.
Depends on: 344048
> Setting browser.tabs.tabMinWidth to a small value (such as 20) and restarting
> Firefox seems to help.

I agree with jesse that setting this pref to a smaller value makes things more usuable.
Assignee: nobody → sspitzer
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Fix for 221684 (tab overflow) is worse than the disease. → usability problems with tab overflow / scrolling
just chatted about usability problems with tab scrolling with pkasting over IRC, and here are his thoughts and suggestions:

See http://wiki.mozilla.org/Tabbed_Browsing/User_Interface_Design/Tab_Overflow If you scroll to the "final thoughts" section of that, you'll see the conclusion there is that the right tab overflow solution was "chevron menu with scrolling strip".  I was just wondering why you didn't do that in your overflow implementation.  Or if you were planning to do that some day but the current scroll strip was a first step

It seems a ton more usable than the scrollbutton solution, and if there wasn't a particular reason it'd been rejected, I'd be interested in helping do it ASAP, preferably for Beta 2

I think we could just show a dropdown at one end of the tab bar, list all tabs in it, and show visually [say, by changing the background color] which tabs were currently onscreen (that would avoid Ben's "two menus suck" complaint, which I agree with)

The reason I ask isn't just because I like old wiki pages I have been trolling msgboards for usecases and stuff And I'm pretty concerned about how scrollstrip + scrollbuttons handles the "doing research" case Where someone may have 20-30 tabs that they need semi-random access to.  This is a case I've run into myself, and a lot of people out there vocalized it...

(concerning the ui in this bug) I would kill both scrollbuttons and put a single dropdown at the right edge of the strip (like the location bar)

Then put all tabs into that dropdown, and use a highlighted background color for whatever set of tabs are currently onscreen.  Clicking any tab in the list selects it and, if necessary, zoomscrolls the strip to it

I would probably hide the dropdown entirely when there is no tab overflow

The reason I like this proposed UI better is:

(1) dropdown targets don't suggest "right-click me", that interaction is unusual
(2) a dropdown arrow on an arrow icon is visually confusing
(3) menus at both ends of the strip sucks, because it's two mouse targets, and you have to remember which end of the strip your desired tab lives on

As for why menus are superior to just having scroll buttons, see that wiki page of ben et. al's commentary on various overflow solutions

The short summary is, it gives you full-length titles for your tabs, random access, instant access, and no need to remember which end your tab is on, all with one easy click target

I wouldn't allow users to DND reorder items inside the menu itself.  I would only allow the tab strip as a DND target, so you can DND to somewhere in your currently visible set of tabs, which is probably good enough

And you can still drag the tab off one side of the strip to auto-scroll drag it, just like now.  as soon as you drag a tab "out of the window" to one side we start scrolling the tab strip.  That is a Fitts' Law win over drag-targeting the arrows

once the user drops it, put the tab at the "appropriate place"*, and then possibly zoom-scroll to it.  (*appropriate place is not clear to me.  Might be after endmost completely visible tab, after endmost partly visible tab, or at end of all tabs in strip)
Status: NEW → ASSIGNED
setting to b2, asking for blocking ff2.
Flags: blocking-firefox2?
Target Milestone: --- → Firefox 2 beta2
I like the idea of having a single menu that shows all open tabs. This way the content of the list is deterministic at all times, which I think would make it less frustrating from a user perspective than a dynamic list. 

I also like Peter's idea of highlighting the visible range. 

I would be very interested to see a patch that implements this.
>  I think we could just show a dropdown at one end of the tab bar

if we do a menu drop down, I agree with peter / ben, that like the location bar (and other UI elements) we just want one of them, on the right (when in ltr mode).

I remember ben telling me about a mac dnd menu issue which might prevent us from dragging tabs onto the menu list of all tabs, to do reordering.  but we could still do dnd reordering (and scrolling), as pkasting suggests.
Other thoughts after talking to darin:

* We need to make sure that "drag off end of strip to scroll" has the windowframe as a target for this action too, so users with maximized browsers can still do it.

* The dropdown should look just like the one on the end of the location bar.

* We should never show a partial tab on the left.  I'm not sure if we should show partial tabs or blank space on the right, when the user doesn't have an evenly divisible strip size.  But we should always scroll such that a tab starts exactly on the "left edge" like it does in 1.5.
More thoughts while I'm thinking:

* The dropdown chevron should probably always be visible and active, rather than hidden/dimmed in the non-overflow case.  This way even people with very small minWidths get the menu benefits, such as seeing a longer tab title

* Perhaps the "different colored background" area should be on the tabs NOT currently visible instead of those currently visible, which might be more intuitive and would probably play more nicely with the first point above.  (see image linked from bug 344171 comment 5, and note how the light area at the bottom looks like "foreground")

* Consider close buttons on the menu? (see image linked from bug 344171 comment 5)

* If we can get DND to the menu working, let's go for it, but it's not a blocker
(In reply to comment #12)
> Other thoughts after talking to darin:
> 
> * We need to make sure that "drag off end of strip to scroll" has the
> windowframe as a target for this action too, so users with maximized browsers
> can still do it.
> 
> * The dropdown should look just like the one on the end of the location bar.
> 
> * We should never show a partial tab on the left.  I'm not sure if we should
> show partial tabs or blank space on the right, when the user doesn't have an
> evenly divisible strip size.  But we should always scroll such that a tab
> starts exactly on the "left edge" like it does in 1.5.
> 

For your last point (partial tab): https://bugzilla.mozilla.org/show_bug.cgi?id=343681

Agree with your point on shading the Tabs NOT visible... makes lot of sense.

Also, as an addendum to my comment in Bug 344171... (Image: http://img101.imageshack.us/img101/8685/firefoxtaboverflowver33cg.png), I was thinking on the lines of greater keyboard integration by popping up the Tab Menu when a user navigates using Ctrl+Tab. Will greatly increase the navigational visibility (especially when Tab size is very small)...thoughts?
If such a drop-down should be the way to go, just make sure that it actually touches the right window border (according to Fitts' law) but that it's to the left of the close button should it be displayed in the Firefox 1.* location.

(In reply to comment #13)
> * The dropdown chevron should probably always be visible and active, rather
> than hidden/dimmed in the non-overflow case.

Maybe with the exception when all tabs are fully visible.

> * Consider close buttons on the menu?

Just make sure to follow the closeButtons pref.

> * If we can get DND to the menu working, let's go for it

And maybe even the inverse case: DND from the menu (which then also gets us DND within the menu) - which I'm usually opposed to (menu items aren't HIG conform), but might make sense in the overflow case.

(In reply to comment #14)
> I was
> thinking on the lines of greater keyboard integration by popping up the Tab
> Menu when a user navigates using Ctrl+Tab. Will greatly increase the
> navigational visibility (especially when Tab size is very small)...

I'd rather make the drop-down list keyboard accessible and leave Ctrl+Tab alone (not slow it down for a doubtful benefit).
Blocks: 343681
(In reply to comment #15)
> If such a drop-down should be the way to go, just make sure that it actually
> touches the right window border (according to Fitts' law) but that it's to the
> left of the close button should it be displayed in the Firefox 1.* location.

Touching the right border might break tab DND off the right edge.  We definitely need to make the button targetable, though, which is a problem with the current scrollbuttons.  I *think* I agree about it being to the left of the closebutton when that's shown, but I can see arguments both ways.

> (In reply to comment #13)
> > * The dropdown chevron should probably always be visible and active, rather
> > than hidden/dimmed in the non-overflow case.
> 
> Maybe with the exception when all tabs are fully visible.

I'm not quite sure what you mean; it sounds like you're directly disagreeing with my point.  I don't think we should disable the menu even when all tabs are fully visible; I don't think it buys us anything to do so.

> > * Consider close buttons on the menu?
> 
> Just make sure to follow the closeButtons pref.

Skeptical.  Close buttons in the menu doesn't seem related to close buttons on tabs to me from a pref point of view.  Let's just decide what the right thing is and do it.  It might be bad to put the close buttons on menu items period.

> And maybe even the inverse case: DND from the menu (which then also gets us DND
> within the menu) - which I'm usually opposed to (menu items aren't HIG
> conform), but might make sense in the overflow case.

I thought about this, but I think it makes the menu harder to use.  (I already think tab DND makes tabs harder to use, in that I accidentally drag instead of clicking quite frequently).  I'd rather be more conformant to users' normal expectations of a menu.

> > I was
> > thinking on the lines of greater keyboard integration by popping up the Tab
> > Menu when a user navigates using Ctrl+Tab. Will greatly increase the
> > navigational visibility (especially when Tab size is very small)...
> 
> I'd rather make the drop-down list keyboard accessible and leave Ctrl+Tab alone
> (not slow it down for a doubtful benefit).

I agree, ctrl-tab doesn't mean "show me a tab switcher", it means "next tab".  Popping up the menu is the wrong thing to do in that scenario.

More thoughts, from last night lying awake:

* When the user adds a new tab to an offscreen portion of the strip, flash the dropdown between one and three times (a bit like the windows taskbar notification flash).  This is visual feedback that the user created a tab and the menu contents have therefore changed in a way that wouldn't otherwise be apparent.  This addresses the last point listed in comment 0.

* The zoom-scroll is going to be tricky to get right (/me remembers the smooth scrolling implementation wars in horror).  I'm open to programs, algorithms, etc. that do this well.  Above all, it needs to be very fast (so you aren't ever "waiting on the program"), but also be an aid to navigation (i.e. actual scrolling instead of just dropping the new tab instantly onscreen).  As a first cut, I was thinking:
** Always select the tab instantly and show its contents in the browser window, so the user doesn't need to wait for the scroll in to begin using the page.
** When the tab is already fully visible, don't scroll at all.
** Otherwise, scroll the tab rapidly into view at some high rate.  Once it begins to come onscreen, ramp down the rate so the tab decelerates to a stop.  The scroll and deceleration must be as smooth as possible.  The deceleration must take a very short period of time to happen, certainly less than half a second.
** I'm not sure whether the tab should stop as the endmost visible tab on the strip, or we should scroll such that one more tab beyond it comes into view.  I think the latter (in overflow cases, that's not much additional distance, and when you're looking at a tab, you often want the next/previous tab aftwerwards; this makes that easier).
** If the user clicks on the tab bar while it is scrolling, immediately select the clicked tab, and start the deceleration to stop the strip as soon as possible.  (I don't think users would actually be able to see and select tabs as they're whizzing past, but it's important that the tab bar feel responsive to their actions.)
** If the click above is on a tab that cannot be decelerated in time to still be fully onscreen, reverse the tab scroll direction as well, so it decelerates back onto the screen.  It should stop in the position we select in my comments two points above (i.e. the endmost or second-endmost position, probably the latter).
Flags: blocking-firefox2? → blocking-firefox2+
there's a couple of threads in the newsgroups about this topic (as you can imagine).  I'll post to the newsgroups about this bug (and the other related bugs) so people are aware of what's going in bugzilla.
notice for safari Version 2.0.3 (417.9.3)

1)  tabs don't scroll
2)  there is the overflow chevron menu
3)  selected tab is "checked"
More thoughts after discussions at lunch:

* Instead of changing background colors in the menu for non-visible tabs, other possibilities include:
** Putting menu separators in at the points corresponding to the borders of the tab strip
** Highlighting the currently selected tab, perhaps using the color the theme uses for the background of the currently selected tab, or the highlight strip at the top of it
** Putting a checkmark on the currently selected tab (easiest to code, may fit user expectations best)

* Consider, at the bottom of the menu, adding a separator and a "Show all" item, which is toggleable.  Defaults off; when toggled on, we treat minWidth as 0.  (This doesn't truly "show all" for people with hideously large numbers of tabs, but it's as close as we can get; other wordings possible).  sspitzer suggests making this setting per-window; I'm not sure how to make that coorperate with remembering the user's preferences.

* Need the UI to look clean at the edge of the tab strip, which the scrollbuttons don't do.  In particular this means playing nicely with the thin, window-width strip at the bottom of the tab strip.

* Need to find a good maxwidth for the dropdown's menu; one possibility is a global menu maxwidth, another is the tab maxwidth

* If there are too many tabs for even a full-height popup, consider a scrollbar along the side a la the location bar dropdown, as opposed to arrows at top and bottom a la right click context menus.

* I apparently didn't make it clear before that, just like the location bar dropdown, this popup should appear after a single click, and go away when you make a selection or click off it.  It should not require you to drag through the menu.
(In reply to comment #20)
> More thoughts after discussions at lunch:

Throwing in late, liking where we're at so far, let's get to the good stuff:

> ** Highlighting the currently selected tab, perhaps using the color the theme
> uses for the background of the currently selected tab, or the highlight strip
> at the top of it
> ** Putting a checkmark on the currently selected tab (easiest to code, may fit
> user expectations best)

I think making the tab title bold and adding a checkmark is all we need. I'm not sure that indicating the "set of currently visible tabs" is an interesting task to users who have clicked on this menu, presumably to "move to / find a tab that I think I should have open somewhere in this mess."

> * Consider, at the bottom of the menu, adding a separator and a "Show all"
> item, which is toggleable.  Defaults off; when toggled on, we treat minWidth 

Hm. Hmmmm. "Show All Tabs" is the text you're looking for, I think. I need to mull this a little, but my gut says that we definitely need this or some other way to revert to the old setting; it's just not sure if about:config won't be good enough for those people.

> tabs, but it's as close as we can get; other wordings possible).  sspitzer
> suggests making this setting per-window; I'm not sure how to make that
> coorperate with remembering the user's preferences.

I don't think it should be per-window anymore than other tabbed browsing settings are. Not to "slippery slope" this argument, but why would this be special from minClipWidth, showing the tab strip, link targeting behaviour, etc.

> * Need the UI to look clean at the edge of the tab strip, which the
> scrollbuttons don't do.  In particular this means playing nicely with the 
> thin, window-width strip at the bottom of the tab strip.

We'd also need the UI to be able to indicate that something (create/delete/request for attention) has happened off in invisible tabstrip land. A basecamp/wordpress like colour pulse or glow seems like a likely candidate for success!

> * Need to find a good maxwidth for the dropdown's menu; one possibility is a
> global menu maxwidth, another is the tab maxwidth

Menu maxwidth ... the tab maxwidth will be set to get more tabs on the screen for one-click selection, not for readbility of the tab names. If the user's gone to the drop-down, they might as well be able to read the tab names.

> * If there are too many tabs for even a full-height popup, consider a 
> scrollbar along the side a la the location bar dropdown, as opposed to arrows 
> at top and bottom a la right click context menus.

Would it be a type-ahead field or look otherwise like a dropdown chooser as opposed to a drop-down menu? If so, then I'd be for this. If not, then we should stick with the same scrolling mechanism that's available in all our other drop down menu rendering.

> * I apparently didn't make it clear before that, just like the location bar
> dropdown, this popup should appear after a single click, and go away when you
> make a selection or click off it.  It should not require you to drag through
> the menu.

I do think we need to be clear on whether this is a popup or a menu; I think menu makes more sense, since users can't really create or provide input, just select from an existing list. Willing to hear from others on the point, though!
In the interest of keeping this from war-by-essay, bullet points as comments:

* Zoom-scroll is a nice-to-have, but not at all required.
* Some indication of the new tab is needed, not sure if flashing the menu is the right thing, but probably is first up 
* We should bump the minTabWidth pref back to 90 or so once we do this so that tab labels are actually still useful (on Mac, 60 is pretty useless)
* Arrows on end can and definitely should go away
* The menu should always be available
* Closebuttons on menu is pretty ugly, and probably not what we want/need
* Show All doesn't have a purpose, unless we think that people will be well-served by that, which I strongly disagree with.  Feels like feature creep to me.
* Having a scrollbar for the popup means inventing new widgetry, instead of just building a menupopup.  We don't have time to work through that type of issue.
* The History menu is 50em, the extra-long label is pretty useful, so I'd default to that.
I found this extension useful. Perhaps some parts of it would be useful here:

LastTab (https://addons.mozilla.org/firefox/112/)
(In reply to Mike Connor, comment #22)

> * Having a scrollbar for the popup means inventing new widgetry, instead of
> just building a menupopup.  We don't have time to work through that type of
> issue.

FWIW, a menulist with some style tweaks could be used here.
(In reply to comment #18)
> Created an attachment (id=228827) [edit]
> for comparison / discussion, how safari Version 2.0.3 (417.9.3) does tab

I really like the simplicity of this approach. Points to consider:
* Only show the overflow/chevron menu when in the overflow case. Opposed to IE7 where it's a menu-button to the left of the tabs, it'd look strange (and IMHO unnecessary cluttered) to have the chevron always at the far right (unless the tabs already reach the right end).
* Make the drop-down menu's items DNDable so that mouse users can move invisible tabs to the front.
* Make the overflow button keyboard accessible through arrow navigation but don't add it to the tabbing order.
* Flash that button three times when a tab has been added invisibly and keep it in the flashed color until the user actually selects it/moves it into visible range.
* And as already mentioned: it should still go to the left of the close button in the Firefox 1-like setup.

This approach would do away with all scrolling issues (overflow in the menu is already handled through scrollbuttons), provide the simplest UI possible and would probably never affect users who never open more than half a dozen of tabs.

(In reply to comment #21)
> I think making the tab title bold and adding a checkmark is all we need.

Please rather use a radio bullet than a checkmark. Checkmarks imply (to me anyway) that several items could be selected at the same time which obviously doesn't hold for tabs.
(In reply to comment #22)
> In the interest of keeping this from war-by-essay, bullet points as comments:
> 
> * Zoom-scroll is a nice-to-have, but not at all required.

Just a little confused (might sound stupid): When you say that Zoom scroll is not required, does it mean that we are NOT going to scroll to a selected (invisible) Tab at all or that we are not going to Scroll in the 'Nice Smooth Way'??
(comment #25)
> * Flash that button three times when a tab has been added invisibly and keep it
> in the flashed color until the user actually selects it/moves it into visible
> range.
That's also discussed in bug 342900 and one thing to consider is that when user opens several tabs in row, there must be some feedback for *each* new tab.

Another observation: when working with tabs, the "tabs of interest" to me are usually the ones around the current one -and- the tabs I recently opened in the background. Motivation:
* I may want to switch to the recently opened tab as soon as it's loaded.
* I may want to see the title of the background tab quickly (e.g. to be able to notice that it was a non-document link, such as javascript: or download link, or if the link just didn't have a descriptive enough text).

None of the solutions mentioned here so far help me in these cases.
(In reply to comment #22)
> * We should bump the minTabWidth pref back to 90 or so once we do this so that
> tab labels are actually still useful (on Mac, 60 is pretty useless)

This only conceals the actual problem that tabMinWidth is independent from the font-family/size. I think it should be an em value.
> Just a little confused (might sound stupid): When you say that Zoom scroll is
> not required, does it mean that we are NOT going to scroll to a selected
> (invisible) Tab at all or that we are not going to Scroll in the 'Nice Smooth
> Way'??

mconnor means scrolling in a 'nice smooth way' is not required.  we'll still ensure the newly selected tab is visible in the tab strip.
(In reply to comment #21)
> I think making the tab title bold and adding a checkmark is all we need. I'm
> not sure that indicating the "set of currently visible tabs" is an interesting
> task to users who have clicked on this menu, presumably to "move to / find a
> tab that I think I should have open somewhere in this mess."

Yeah, I've become convinced by this point of view.  Indicating the current tab is "good enough" to know where you are, and far leass confusing than indicating the visible set.

> > * Consider, at the bottom of the menu, adding a separator and a "Show all"
> > item, which is toggleable.  Defaults off; when toggled on, we treat minWidth 
> 
> Hm. Hmmmm. "Show All Tabs" is the text you're looking for, I think. I need to
> mull this a little, but my gut says that we definitely need this or some other
> way to revert to the old setting; it's just not sure if about:config won't be
> good enough for those people.

I think there are also legitimate circumstances where you need to get at a lot of tabs at once and you're willing to live with something less than tabMinWidth temporarily.  If that's true it would be much better to figure out what those use cases are and how to better solve them; this is a hack in that sense.  But I think it's a useful hack which has little impact on the UI and will be helpful to a lot of people, so I'm willing to live with it for now.

> > tabs, but it's as close as we can get; other wordings possible).  sspitzer
> > suggests making this setting per-window; I'm not sure how to make that
> > coorperate with remembering the user's preferences.
> 
> I don't think it should be per-window anymore than other tabbed browsing
> settings are. Not to "slippery slope" this argument, but why would this be
> special from minClipWidth, showing the tab strip, link targeting behaviour,
> etc.

Well, the counterargument would be that by putting this in the bottom of the tab menu and not in a pref dialog somewhere, you're clearly indicating that the setting affects this group of tabs.  Having it then affect a different group of tabs in another window is pretty unintuitive.  What you're effectively doing then isn't changing how this set of tabs is presented, but toggling a global pref by means of a local menu.  Weird.

I can see technical issues either way, but I'm more concerned about the usability ones.

> We'd also need the UI to be able to indicate that something
> (create/delete/request for attention) has happened off in invisible tabstrip
> land. A basecamp/wordpress like colour pulse or glow seems like a likely
> candidate for success!

Yep, see comment 16.  Looks like we're in agreement.

> > * Need to find a good maxwidth for the dropdown's menu; one possibility is a
> > global menu maxwidth, another is the tab maxwidth
> 
> Menu maxwidth ... the tab maxwidth will be set to get more tabs on the screen
> for one-click selection, not for readbility of the tab names. If the user's
> gone to the drop-down, they might as well be able to read the tab names.

I too wanted to have a wide menu... sspitzer was concerned about the menu "feeling too big".  Let's initially use the menu maxwidth (which is pretty large AFAIK) and see how it looks.  (Although I don't know what "menu maxwidth" really means; mconnor says the History menu uses 50 em, I don't know if that's larger or smaller than a global max).

> > * If there are too many tabs for even a full-height popup, consider a 
> > scrollbar along the side a la the location bar dropdown, as opposed to arrows 
> > at top and bottom a la right click context menus.
> 
> Would it be a type-ahead field or look otherwise like a dropdown chooser as
> opposed to a drop-down menu? If so, then I'd be for this. If not, then we
> should stick with the same scrolling mechanism that's available in all our
> other drop down menu rendering.

Sigh, I guess.  (See also bug 275658.)  OS consistency is good, even if in this case I think it's less usable.

> > * I apparently didn't make it clear before that, just like the location bar
> > dropdown, this popup should appear after a single click, and go away when you
> > make a selection or click off it.  It should not require you to drag through
> > the menu.
> 
> I do think we need to be clear on whether this is a popup or a menu; I think
> menu makes more sense, since users can't really create or provide input, just
> select from an existing list. Willing to hear from others on the point, though!

Well, the reasons I had wanted it a popup were:
* Makes close-buttons-in-popup possible; but maybe that isn't a good idea
* Makes within-menu-DND possible; but that may not be a good idea OR technically possible
* Makes it easier to spend a minute perusing this very long list of items

However, if we throw out points 1 and 2, I don't think point 3 is a good reason on its own, and a menu would probably make more sense.

(In reply to comment #22)
> * Zoom-scroll is a nice-to-have, but not at all required.

I agree, this isn't a blocker, it's a second-tier item.

> * We should bump the minTabWidth pref back to 90 or so once we do this so that
> tab labels are actually still useful (on Mac, 60 is pretty useless)

Also agree, see discussion on bug 344048.  60 was due to knowing our current solution sucked, for B2 we should do "the right thing".

> * Closebuttons on menu is pretty ugly, and probably not what we want/need

My gut feeling is that this is right.

> * Show All doesn't have a purpose, unless we think that people will be
> well-served by that, which I strongly disagree with.  Feels like feature creep
> to me.

I could be convinced either way.  As I said above, I'd like some concrete use cases why this would be helpful to people besides those who just hate the tabMinWidth.

(In reply to comment #25)
> * Only show the overflow/chevron menu when in the overflow case. Opposed to IE7
> where it's a menu-button to the left of the tabs, it'd look strange (and IMHO
> unnecessary cluttered) to have the chevron always at the far right (unless the
> tabs already reach the right end).

As I said in comment 13 and comment 16, I don't think this is the right way to go.  The tab close button didn't "look cluttered" (it was removed for usability reasons, not clutter), and the location bar doesn't "look cluttered".  And I don't think there are usability reasons why it's good to not have this menu available in the non-overflow case; on the contrary, I might want it to read long titles even IN a non-overflow (especially near-overflow) case.

> > I think making the tab title bold and adding a checkmark is all we need.
> 
> Please rather use a radio bullet than a checkmark. Checkmarks imply (to me
> anyway) that several items could be selected at the same time which obviously
> doesn't hold for tabs.

I'd be happy enough with that, seems like a good point.

(In reply to comment #27)
> > * Flash that button three times when a tab has been added invisibly and keep it
> > in the flashed color until the user actually selects it/moves it into visible
> > range.
> That's also discussed in bug 342900 and one thing to consider is that when user
> opens several tabs in row, there must be some feedback for *each* new tab.

I like the idea on bug 342900 of flashing the button a highlight color and then fading it back to its normal color.  This has two advantages over the "flash and stay colored" method:
(1) It allows us to indicate each new open tab as the above comment requests
(2) By fading out, it indicates that nothing special/different will happen to the menu when the user opens it.  Contrast with Windows XP's Start menu highlighting, where a highlighted entry means windows is going to highlight the new entries inside.  We're not going to highlight newly opened tabs in the menu, so we shouldn't have the menu be a different color normally.

> Another observation: when working with tabs, the "tabs of interest" to me are
> usually the ones around the current one -and- the tabs I recently opened in the
> background.
> 
> None of the solutions mentioned here so far help me in these cases.

While I'm sympathetic, my suspicions are that these are more general problems around "where should newly opened tabs go", "how should ctrl-tab navigation work", etc., and are beyond the scope of the tab overflow solution.  I don't think I'm prepared to tackle them here so I'd rather leave them for other bugs to stay more focused.
(In reply to comment #27)
> (comment #25)
> > * Flash that button three times when a tab has been added invisibly and keep it
> > in the flashed color until the user actually selects it/moves it into visible
> > range.
> That's also discussed in bug 342900 and one thing to consider is that when user
> opens several tabs in row, there must be some feedback for *each* new tab.
> 
> Another observation: when working with tabs, the "tabs of interest" to me are
> usually the ones around the current one -and- the tabs I recently opened in the
> background. Motivation:
> * I may want to switch to the recently opened tab as soon as it's loaded.
> * I may want to see the title of the background tab quickly (e.g. to be able to
> notice that it was a non-document link, such as javascript: or download link,
> or if the link just didn't have a descriptive enough text).
> 
> None of the solutions mentioned here so far help me in these cases.


This extension http://hashcolouredtabs.mozdev.org/ at least groups ones from similar sites together.
In reply to comment #0)
> In fact, without the feed back provided by the tabs resizing, it becomes 
> difficult to know if you have made a new tab or not.

I think the proposed overflow dropdown menu should rapidly flash and remain highlighted when a new tab is opened off screen. Repeat as necessary for each additional tab opened in rapid succession.  The highlighting should remain until the dropdown is activated.  

Optionally, (on mouseover?) the highlighted dropdown should display a tooltip stating "A new tab has loaded off screen." which fades away after a few seconds.

(In reply to comment #27)
> * I may want to see the title of the background tab quickly (e.g. to be able
> to notice that it was a non-document link, such as javascript: or download
> link, or if the link just didn't have a descriptive enough text).

Option: As each off screen new tab finishes loading, display a tooltip under the highlight dropdown stating "Opened: <page title>" which fades away after a few seconds. (If this is done then the option above would definitely be on mouseover only.)

(In reply to comment #20)
> * Need to find a good maxwidth for the dropdown's menu;

Option: Scroll extra long menuitem text on mouseover.  I've seen some windows programs do this and it might be useful for pages with similar but long titles.
(In reply to comment #21)
> Would it be a type-ahead field or look otherwise like a dropdown chooser as
> opposed to a drop-down menu? If so, then I'd be for this. If not, then we
> should stick with the same scrolling mechanism that's available in all our
> other drop down menu rendering.
> 

We've just concluded that scroll buttons don't work well. Surely replacing
them with vertical ones completely defeats the object of changing this.
Scrollbars are far more efficient than scroll buttons for scrolling through
a list...

And actually, users are much more used to scrollbars than they are
to vertical scroll buttons. Show a granny a scrollbar and she'll be able to
use it, she won't care that it happens to be on a popup. Put two tiny, hard
to use buttons at the top and bottom and she'll either not notice them or
struggle with their quirky behaviour.

I know the bookmarks menu does this, but that's a usability nightmare.
Not only does it take ages to reach the bookmark you want, but you have no
idea how far you've scrolled, and once you actually get close it's hard to
actually read the titles because they're now scrolling too fast.

(In reply to comment #22)
> * Having a scrollbar for the popup means inventing new widgetry, instead of
> just building a menupopup.  We don't have time to work through that type of
> issue.
>

All it needs is hiding the two buttons and setting overflow:auto. My Too Many Tabs extension offers this mode.

> * We should bump the minTabWidth pref back to 90 or so once we do this so that
> tab labels are actually still useful (on Mac, 60 is pretty useless)
> * Show All doesn't have a purpose, unless we think that people will be
> well-served by that, which I strongly disagree with.  Feels like feature creep
> to me.
>

I expect a lot of people to complain unless we compromise on one of these. And
the show all tabs functionality could sometimes be quite useful...


(In reply to comment #30)
> Well, the reasons I had wanted it a popup were:
> * Makes close-buttons-in-popup possible; but maybe that isn't a good idea
> * Makes within-menu-DND possible; but that may not be a good idea OR
> technically possible
> * Makes it easier to spend a minute perusing this very long list of items
> 

Within-menu-DND is both possible and useful, e.g. the bookmarks menu and the
bookmarks toolbar overflow menu (see also the Windows start menu)
in that screen shot, I'm trying out:

1)  the selected tab is bold
2)  menuseparator before (and after) the selected tab assuming there is another tab before (or after) the selected tab
3)  max-width is 42em, as defined for all menu/menuitems in popups here:  http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/pinstripe/global/menu.css#127
(In reply to comment #32)
> Optionally, (on mouseover?) the highlighted dropdown should display a tooltip
> stating "A new tab has loaded off screen." which fades away after a few
> seconds.

I don't think this is very useful (if you're already hovering the menu, you know there's something in it or else can click it), and could potentially be very annoying.

> Option: As each off screen new tab finishes loading, display a tooltip under
> the highlight dropdown stating "Opened: <page title>" which fades away after a
> few seconds. (If this is done then the option above would definitely be on
> mouseover only.)

Again, I'm skeptical that anything more than a notification flash on the dropdown is necessary for this.  Tooltips, text, etc. are very irritating when you didn't want them.

> Option: Scroll extra long menuitem text on mouseover.  I've seen some windows
> programs do this and it might be useful for pages with similar but long titles.

Like, continuous horizontal scroll a la winamp?  I think we should provide a menu that's wide enough to see "as much as you need" of the title and avoid this scrolling.  The downside seems bigger than the upside, since if you have the time to mouseover the item, wait for it to scroll, and read it, you might as well have just selected the tab to pull it up.

(In reply to comment #33)
> We've just concluded that scroll buttons don't work well. Surely replacing
> them with vertical ones completely defeats the object of changing this.
> Scrollbars are far more efficient than scroll buttons for scrolling through
> a list...

It doesn't completely defeat it, for several reasons:
* The dropdown can show many more entries than the tab strip, pushing the scrollbar case out to edge cases
* The scroll arrows on dropdowns are MUCH bigger targets than the old scroll arrows, which was a major headache of the old solution
* You can still easily navigate (n / 2) tabs away with a dropdown without scrolling (where n is the maximum height of the dropdown), as opposed to 0 with the old scroll solution

So let's not get TOO hasty in our disappointment here.

> And actually, users are much more used to scrollbars than they are
> to vertical scroll buttons. Show a granny a scrollbar and she'll be able to
> use it, she won't care that it happens to be on a popup. Put two tiny, hard
> to use buttons at the top and bottom and she'll either not notice them or
> struggle with their quirky behaviour.

This is how all windows popups have worked since pretty much the dawn of time, so I question whether your description of user confusion is accurate.  A shining example is what happens when you add more entries to Start->Programs than fit on the screen vertically (as many, MANY people I know have done).

> > * Show All doesn't have a purpose, unless we think that people will be
> > well-served by that, which I strongly disagree with.  Feels like feature creep
> > to me.
> 
> I expect a lot of people to complain unless we compromise on one of these. And
> the show all tabs functionality could sometimes be quite useful...

Again, I'm not necessarily disagreeing, but give me use cases.
(In reply to comment #35)
> 1)  the selected tab is bold

I like that

> 2)  menuseparator before (and after) the selected tab assuming there is another
> tab before (or after) the selected tab

I'm not so sold on this.  It makes it very clear what tab you're on, but it adds space to the menu and makes it look like clicking the tabs in the "first", "second", and "third" menu groups will maybe do different things.  I'd like to see it with a bullet (a la radio buttons) next to it instead.

> 3)  max-width is 42em, as defined for all menu/menuitems in popups here: 

That max width is probably big enough, at least for a first cut.
> I'm not so sold on this.

me neither (and neither was robert strong).  But worth trying out.

Here comes a new screen shot, without the separators, but with the default icon for tabs without icons, from chrome://global/skin/icons/folder-item.png)
(In reply to comment #35)
> 2)  menuseparator before (and after) the selected tab assuming there is another
> tab before (or after) the selected tab

I think changing the background-color or adding a semi-transparent background-image would look cleaner.

(In reply to comment #37)
> I'd like to
> see it with a bullet (a la radio buttons) next to it instead.

bullet + favicon isn't possible, is it?
(In reply to comment #40)
> Created an attachment (id=229012) [edit]
> screen shot, without separators

Loss of clarity.
(In reply to comment #39)
> (In reply to comment #35)
> > 2)  menuseparator before (and after) the selected tab assuming there is another
> > tab before (or after) the selected tab
> 
> I think changing the background-color or adding a semi-transparent
> background-image would look cleaner.

I agree... simple (light background) shading of the selected Tab is all that is needed IMO. In that case we will not have to worry about 'Radio Button + Favicons' etc kind of scenarios... plus shading is pretty distinctive and serves the purpose well.
(In reply to comment #39)
> (In reply to comment #37)
> > I'd like to
> > see it with a bullet (a la radio buttons) next to it instead.
> 
> bullet + favicon isn't possible, is it?

It had better be or we're reduced to menuseparators or the background color solution, neither of which is as good IMO.

There are numerous reasons I was convinced that colors aren't as good for this, including
* They become invisible when you mouseover and thereby highlight the item
* They're hard to select such that they
** Play nicely with the theme and the user's system colors
** Still allow readability

I mean, they may be doable if we have to, but I'd rather not.

OK, one other thing that just came up in #bonecho that I wanted to mention is: we should consider exposing varous tab-navigation keyboard shortcuts on this menu (just like shortcuts exposed on other menus).  Off the top of my head we have alt+1-8 for the first 8 tabs, alt+9 for the last tab, ctrl+tab for next tab, shift+ctrl+tab for previous tab.  Some of these, obviously, might overlap; we'd have to pick the "best shortcuts" to show.
(In reply to comment #43)
> (In reply to comment #39)
> > bullet + favicon isn't possible, is it?
> 
> It had better be or we're reduced to menuseparators or the background color
> solution, neither of which is as good IMO.
> 
> There are numerous reasons I was convinced that colors aren't as good for this,

On the other hand, two icons would probably look bloated.
(In reply to comment #44)
> On the other hand, two icons would probably look bloated.

There won't be two icons.  There will be an icon and a bullt, which from a user perspective isn't quite the same thing.  Also the bullet will be all alone out in its own vertical column.

Even if we don't have a working patch, if someone could photoshop it up, we can test this claim pretty quickly.
(In reply to comment #45)
> Also the bullet will be all alone out
> in its own vertical column.

This depends on the theme.
(In reply to comment #42)
> (In reply to comment #39)
> > (In reply to comment #35)
> > > 2)  menuseparator before (and after) the selected tab assuming there is another
> > > tab before (or after) the selected tab
> > 
> > I think changing the background-color or adding a semi-transparent
> > background-image would look cleaner.
> 
> I agree... simple (light background) shading of the selected Tab is all that is
> needed IMO. In that case we will not have to worry about 'Radio Button +
> Favicons' etc kind of scenarios... plus shading is pretty distinctive and
> serves the purpose well.

Maybe a combination of the coloring from both of these extensions would do it:

Colorful tabs
http://varun21.googlepages.com/main.html

Hash Colored Tabs
http://hashcolouredtabs.mozdev.org/

Attachment #229005 - Attachment is obsolete: true
Attachment #229012 - Attachment is obsolete: true
Attached image screen shot of radio (instead of check) (obsolete) —
simon points out:

"Please rather use a radio bullet than a checkmark. Checkmarks imply (to me
anyway) that several items could be selected at the same time which obviously
doesn't hold for tabs."

he does have a point.

note, in safari, in the tab overflow, they use a check.

can someone point to another menu in firefox (or another app) where a bullet is used?
> can someone point to another menu in firefox (or another app) where a bullet is
> used?

View | Character Encoding and View | Page Style in Firefox have it too.

I'm sold on the bullet over the checkmark.
(In reply to comment #50)
> Created an attachment (id=229036) [edit]
> screen shot of a radio menu item, from tbird
> 

Having seen the three screen shots, a few observations:

- The radio button does not seem pronounced enough... difficult to make out. 'Check' seems to be the better option here and looks clean enough.
- Looking at the Menu, I am feeling more and more that we need some kind of shading, of either the visible or the invisible tabs. Reason being that if I have a large number of tabs open (>30-35) its likely that I am working with several different groups of tabs... and when I intend to work with a particular group of tabs, I would ideally want all of them in the visible zone for quick (one-click) access... without shading it is visually more difficult to locate & determine if the group of tabs that I intend to work with is within the visible or invisible zone and which particular tab should I be selecting in order to bring the particular group of tabs into visible zone. Example: I have 40 tabs open and I have Tab Nos. 1-12 in the visible zone. I have a group of tabs from No. 31-38 that I intend to work with and hence I want to bring them into the visible zone. Ideally I would select Tab No. 38 so that all tabs from No. 31-38 will come into the visible zone, but without some kind of shading/indication, its difficult to determine from the plain menu (mental calculation required) whether Tab No. 38 is currently in the visible zone or not. Hence I would very sincerely request you to consider some kind of visual indication of currently visible/invisible tabs. A mock-up might help here in order to appreciate the the difference (I am currently in office, hence can't create a mock-up myself right now).


(In reply to comment #52)
> Hence I would very
> sincerely request you to consider some kind of visual indication of currently
> visible/invisible tabs. A mock-up might help here in order to appreciate the
> the difference

https://bugzilla.mozilla.org/attachment.cgi?id=228740
(In reply to comment #48)
> Created an attachment (id=229031) [edit]
> screen shot, no more scroll buttons, bold and checked selected tab
> 

(In reply to comment #49)
> Created an attachment (id=229035) [edit]
> screen shot of radio (instead of check)

(In reply to comment #50)
> Created an attachment (id=229036) [edit]
> screen shot of a radio menu item, from tbird
> 

I prefer the check to the radio, but the radio in Thunderbird seems larger, so it is hard to tell. Anyhow, the thought of offering 

Why not just BOLD the text and no radio or check? Also, in the second one here, the tab isn't different at the top, only indicated by the radio as far as I could see.
(In reply to comment #52)
> (In reply to comment #50)
> > Created an attachment (id=229036) [edit]
> > screen shot of a radio menu item, from tbird
> > 
> 
> Having seen the three screen shots, a few observations:
> 
> - The radio button does not seem pronounced enough... difficult to make out.
> 'Check' seems to be the better option here and looks clean enough.
> - Looking at the Menu, I am feeling more and more that we need some kind of
> shading, of either the visible or the invisible tabs. Reason being that if I
> have a large number of tabs open (>30-35) its likely that I am working with
> several different groups of tabs... and when I intend to work with a particular
> group of tabs, I would ideally want all of them in the visible zone for quick
> (one-click) access... 

See my previous posts about the coloring extensions. Say you have 3 mozilla tabs open, and they are all green, and 3 google tabs open which are all blue, and 3 tabs from massbackwards which are red. The tab you are on will be a brighter red, and you will quickly be able to distinguish from different sites if nothing else. I like the way the colored tabs extension colors them, but there is no rhyme or reason type of logic as to which ones get colored, but THAT function is available from the Hash Colored Tabs extension. If the developers here could grab the shading characteristics of the one and the logic of the other, and then use a bold text instead of the check or radio button, I think that would make things clear, simple, and easy for 99.9% of the people here.
> the radio in Thunderbird seems larger

it is larger, but not on purpose.

I used the wrong "radio bullet" image in my last screen shot.  I'll attach a new one.
(In reply to comment #56)
> > the radio in Thunderbird seems larger
> 
> it is larger, but not on purpose.
> 
> I used the wrong "radio bullet" image in my last screen shot.  I'll attach a
> new one.
> 

The larger one is preferred.
Attachment #229031 - Attachment is obsolete: true
Attachment #229035 - Attachment is obsolete: true
Attachment #229036 - Attachment is obsolete: true
(In reply to comment #58)
> Created an attachment (id=229119) [edit]
> bigger radio, more spacing...
> 

This looks good enough now I guess. 
On the other point, like I said before, I really think we should consider shading either visible or invisible tabs coz when the number of tabs is very high, visual identification will be of much use to locate as to how much to the left or right are the tabs that I want to access and bring into view. (Please refer the scenario explained in Comment #52)
> I really think we should consider shading either visible or invisible tabs

I'm working on that now, to see what people think.

Screen shot coming...
(In reply to comment #58)
> Created an attachment (id=229119) [edit]
> bigger radio, more spacing...

Could we actually make the "radio" even bigger?  I'm thinking a full-sized bullet, like 2/3 the diameter of the favicon or something.  The current tab still doesn't look visually distinct enough.

I, too would like to see various color-shading options.  Specifically:
* Shade visible tabs
* Shade non-visible tabs
* Shade current tab
* Shade non-current tabs
* Shade visible tabs, also shade (in yellow or other hightlight color) current tab
* Shade non-visible tabs, also shade current tab

Also, as a side note to people linking to extensions: if you want to discuss a topic, take a screenshot or make a mockup and make clear what in the screenshot/mockup you're referring to.  I'm not going to go download random tab-management systems and play with them just so I can read your comments, I just don't have time.  Please make life easier on me :)
Attached image screen shot
my personal preference (shared by robert strong) is that is that 42ems is too big, see this screen shot for how it looks on 1024 x 768.

Note, I plan to hook it up so that on mouse over the urls in the "all tabs" menu shows up in the status bar of the browser, like we do with the bookmarks menu.
Attached image colorful tabs
This is a screenshot showing what Colorful Tabs extension does. I like the coloring, but there is no logical grouping by color. I'll turn that extension off and do a shot of the same tabs with Hash Colored Tabs doing its thing.
Note how this extension colors tabs from the same site in the same color. The coloring is not as fancy or nice, but it has the logic. 

I'd like to see a combination of these two extensions to color the tab with logic, but do it nicely. Also note in the previous one, the colorful tabs extension had the current tab darker (brighter? not as faded?) as the other tabs.
(In reply to comment #63)
> I like the coloring

I think you should realize that something like this will hardly be part of a Firefox release. It's most suitable for an extension, but not for all users. Not to mention that it isn't related to the problems Seth is trying to solve at the moment.

(Just my two cents, go ahead if you like.)
(In reply to comment #62)
> Created an attachment (id=229144) [edit]
> screen shot
> 
> my personal preference (shared by robert strong) is that is that 42ems is too
> big, see this screen shot for how it looks on 1024 x 768.

I'm not sure.  I think it's OK as is, but would definitely be too big if we added keyboard shortcuts into the menu, because they'd be too far away from their tab titles on the left.  The question, though, is why the global menu maxwidth is 42 em.  Certainly if this really _is_ too wide, that's too wide, right?  So we should figure out why that's set the way it is and see if the logic behind that choice guides this.

I certainly would not make the menu much less than, say, 32 em wide.

(In reply to comment #64)
> I'd like to see a combination of these two extensions to color the tab with
> logic, but do it nicely. Also note in the previous one, the colorful tabs
> extension had the current tab darker (brighter? not as faded?) as the other
> tabs.

OK, thanks very much for the screenshots, that was helpful.  As far as I can tell, this is fairly orthogonal to the tab overflow work going on in this bug, so it should probably be in a separate bug, unless I'm missing something.  I think regardless of how/whether tabs are colored, we can just make the overflow menu match the tab strip.

Which leads me to an idea...

Seth, for your "coloring the menu items" prototype, can you make the colors match the theme colors for the foreground and background tabs?  So make the menu background color for the foreground tab be the same as the color of the foreground tab on the tab strip, and similarly for background tabs.

I don't know how these are currently set in CSS, we might need to rewrite the way the rules of things work to get it so both places change simultaneously when you change one color... that's probably a lower-priority issue though.
Don't know if this screenshot if fake or not, but it handles the shading of tabs in a nice way: http://img291.imageshack.us/img291/5848/fx2newthemeinxpv12ot.jpg
(In reply to comment #67)
> Don't know if this screenshot if fake or not, but it handles the shading of
> tabs in a nice way:
> http://img291.imageshack.us/img291/5848/fx2newthemeinxpv12ot.jpg

http://wiki.mozilla.org/FX2_Visual_Update/Default_Theme_Update

Yes, it does look nice. Yet I don't see how it could be adapted to the overflow menu.
> (In reply to comment #67)
> > Don't know if this screenshot if fake or not, but it handles the shading of
> > tabs in a nice way:
> 
> Yes, it does look nice. Yet I don't see how it could be adapted to the overflow
> menu.

Indeed.  This is not a bug for general discussion of all tab navigation, opening order, coloring, drag and drop, etc. issues.  Please restrict comments in this bug to issues specifically related to tab overflow and a scrollable tab strip, thanks.
The suggestions for coloring came from the request to "group" tabs visually. I'll do up a new bug for coloring tabs.
OK, I've gone through the whole bug and condensed everything down to a sorted list of what should be implemented here.  This should provide an easier reference for implementing.  Note that some of the items (mostly P1 items) are already complete in Seth's checkout.
(In reply to comment #71)
Thank you for the excellent summary.

I followed the discussion and I agree with most of the points. However, I would be hesitant to make the menu button present all the time. Let us not forget that the amount of tabs which forces users to employ tab overflow is an edge case. Assuming a minimal width of a tab around 120px, that case in not hit at all by majority of users. I don't believe it's worth it to bloat the main browser screen with an additional item which is pretty much redundant most of the time and for most of the users.  It would seem inconsequent, considering how conservative we used to be about adding new stuff to the main view because of our struggle to keep it simple.
This updated version adds clarity, reworks the priorities of one or two things, unifies grammar and numbers all the bullets for easier referencing of individual points.
Attachment #229174 - Attachment is obsolete: true
This differs from v2 in two important ways:
(1) Annie Sullivan pointed out that all other popup/dropdown/whatever elements she could find in Firefox are click-to-dismiss rather than release-to-dismiss.  See for example the behavior of the back button's history pulldown.  I think being consistent here is critical, so I have changed that part of the spec.  This has enabled some Drag-And-Drop items to move out of the Rejected list into the P3 list, hooray.
(2) Comments by multiple people, looking at IE7, and my original gut feeling have brought me back around to thinking that indicating the visible set of tabs is a good idea.  I've swapped the items regarding popup coloring around to indicate this.

Seth, I'm interested in two things:
(1) An implementation of all P1 items ASAP so this can get reviewed and on trunk immediately.  In an ideal world we could have a patch to start reviewing by the end of tomorrow.  I think a lot of the P1 stuff describes things that have already been done, so hopefully this isn't too much more work.  This lets people begin playing with the bare, clunky functionality immediately.
(2) Implementation of 2A-2D ASAP after item (1).  The prerequisite here is figuring out the best way of doing item 2C.  Getting all this stuff into trunk along with (1), or shortly after, would be a Good Thing.

The rest of the items on my list can happen after the above.
Attachment #229187 - Attachment is obsolete: true
> Rejected Ideas
>   RA. Drop down uses scrollbar instead of scroll arrows when too tall to
>       completely show - clashes with OS conventions

Interestingly, Internet Explorer 7 uses a popup with a scrollbar for favorites/feeds/history (see attachment). The concept is of it being a temporary sidebar (in fact it can optionally be docked).
This seems to be Vista's prefered way of doing things: even the Start Menu All Programs section uses a scrollbar.
The resulting UI is very slick and effective.

Also I dispute your point that as the popup can show a lot of tabs, the ineffectiveness of the scroll arrows isn't significant (paraphrasing a little): The popup can show very few more tabs than could be shown by Firefox 1.5 and below, and an important part of the need to fix this is that seeing just that many tabs wasn't enough.


(In reply to comment #74)
> (1) Annie Sullivan pointed out that all other popup/dropdown/whatever elements
> she could find in Firefox are click-to-dismiss rather than release-to-dismiss. 
> See for example the behavior of the back button's history pulldown.  I think
> being consistent here is critical, so I have changed that part of the spec. 

As we are going for a popup then, that raises the possibility of letting right-clicking on a tab in the popup show the standard tab right-click menu. This would be very handy, and consistent with the way the bookmarks menus operate.
Obviously this is low priority though.

P.S. Thanks for a very useful summary.
I've modified my local draft of the design doc based on these comments, so they'll be reflected next time I upload something.

(In reply to comment #75)
> Interestingly, Internet Explorer 7 uses a popup with a scrollbar for
> favorites/feeds/history (see attachment). The concept is of it being a
> temporary sidebar (in fact it can optionally be docked).

This gave me an idea to make it possible to "dock the dropdown", which would remove the tab strip and instead show the list of tabs in a sidebar.  I think it's questionable and probably also beyond the scope of this, but I stuck it in the Controversial section.

> This seems to be Vista's prefered way of doing things: even the Start Menu All
> Programs section uses a scrollbar.
> The resulting UI is very slick and effective.

I don't disagree that scrollbars are easier to use.  Added comments about Vista to this item in the Rejected section.

> Also I dispute your point that as the popup can show a lot of tabs, the
> ineffectiveness of the scroll arrows isn't significant (paraphrasing a little):
> The popup can show very few more tabs than could be shown by Firefox 1.5 and
> below, and an important part of the need to fix this is that seeing just that
> many tabs wasn't enough.

I think you're crossing up a couple different bits of argument here:
* My comparison of "items shown" was for a dropdown versus the tab-strip-with-reasonable-minWidth case, not against the Firefox 1.5 case.
* The problem with the Firefox 1.5 solution was that there was no overflow solution at all.

My argument was that the proposed solution is much better than the B1 solution, not that (with huge numbers of tabs) it is much better than the 1.5 solution.  With huge numbers of tabs, the new solution is better than 1.5 not because it can show you more tabs at once, but because it can show you more of the titles for them, and because it allows overflow _at all_.  And once again, you can still navigate n/2 tabs at once without scrolling, so the only case where this becomes merely marginally better than 1.5 is in the "quickly jump between the ends of the list when I have a huge number of tabs" case.  And _that's_ the reason why we have keyboard shortcuts that immediately take you to the first and last tab, and I want to expose them on this menu.

> As we are going for a popup then, that raises the possibility of letting
> right-clicking on a tab in the popup show the standard tab right-click menu.
> This would be very handy, and consistent with the way the bookmarks menus
> operate.

Good idea, added to the P3 section.
Quote from 'Prioritized design for new solution, v3'
>>>  1M. In its static state, the tab strip only shows complete tabs.  A partial
      tab never appears at either end of the strip (unless the strip is
      scrolling [see item 3G]).  If the strip width is not divisible by the tab
      width, padding is inserted between the rightmost visible tab and the menu
      button, so there is always a tab beginning at the very left edge of the
      strip.
  1N. The tab strip shows as many tabs as possible.  The space between the
      rightmost tab and the menu button is never large enough to hold a
      complete tab.
>>>

If we are looking for a 'Clean' solution to avoiding tab-cropping in the tab bar, I don't think 'padding' the extra space is the cleanest implementation. This discussion has been a part of Bug 343681 ... the idea basically being that whatever maximum number of tabs can fit the tab bar should be expanded to fit the tab bar instead of introducing a padding (I guess this too has been implemented in IE7). So if 15 and a half tabs can fit the tab bar, the 'half tab' moves to the overflow section and 15 tabs are expanded in size appropriately to exactly fit the tab bar...
Peter, great summary in attachment 229193 [details]. Comments from the ui-dude ..:

1F - will need to co-ordinate with bug 328065, added that one as a dependency

1M - I actually disagree with this; partially cut off tabs add a subtle visual cue that lets users know when they do have more tabs open than are currently displayed  in the tabstrip - assuming 1C, it seems important to me that we have this cue

1N - I think our current minWidth (2.0b1 uses 60px) is too narrow actually; we want to be able to hold the favicon + " - " + "A word", which by my playing seems to be closer to 85px or 95px. This value is likely to be highly contentious but, thankfully, is easy to play around with.

2A - would promote this to a P1 item, myself; favicons significantly aid in the  quick identification of the desired target

2C - The IE7 solution highlights the currently focused tab, and as mentioned in comment 21, I don't think this is needed; I'd put this in the Rejected section.

2G - we're hoping to support the creation of new windows by dragging a tab offscreen in Firefox 3. I'd rather see something like the scrollspeed accelerating significantly the longer the mouse is held off the end of the tabstrip, or keyboard support for responding to Accel-1/Accel-9 while dragging a tab

2L - redundant with 1N?

3B - I was on the fence in comment 21, but I think I've convinced myself that this setting should remain either as a pref or a hidden pref; I can't see the usefulness of the toggle case, and it disrupts the "scroll-to-the-end" operation and conflicts with the "Open in Tabs" item we have elsewhere

3G - love it; need to ensure the entire operation completes in > 0.75s

3H - I don't think we need to bother with this; the animation should be too quick to actually make this useful. Or were you thinking of a cancel use case?

UB - I think option 0 makes the most sense here, or, if you want to get really fancy, you could figure out the tab index of the currently selected tab and make it so that the selected tab is in that same position (ie: if I'm on tab number 2 of 7 visible, and I select number 9 from the list, then 8-15 are shown with 9 selected)

UD - yes

RA - someone mentioned that IE7 does this; lies. The IE7 box isn't a drop-down menu, it's a pop-out overlay that can be pinned as a sidebar. We're talking about a drop-down menu here, and should work like one.

Again, great work to bring this together, Peter. 
Depends on: 328065
(In reply to comment #77)
> if 15 and a half tabs can fit the tab bar, the 'half
> tab' moves to the overflow section and 15 tabs are expanded in size
> appropriately to exactly fit the tab bar...

This has problems with very odd-looking behavior on window resize and isn't consistent with the case where you have few tabs open.

The only case where this actually kicks in is when your tabs have hit their minimum width.  In that case, tabs are fairly small, and having them expand and contract slightly as you resize the window just looks goofy, I think.

I could probably be convinced by playing with working mockups of each and finding that the stretchy behavior was actually better, but short of that I'm very skeptical.

(In reply to comment #78)
> 1M - I actually disagree with this; partially cut off tabs add a subtle visual
> cue that lets users know when they do have more tabs open than are currently
> displayed  in the tabstrip - assuming 1C, it seems important to me that we have
> this cue

I've waffled on this before.
Cons:
* 1F is much harder (in B1 this looks terrible).  Maybe we could show a "jagged edge" of the tab
* If there are actually tabs users don't know are out there, they should be caught by 2I
* Also, given 2C, the dropdown itself will provide this cue
* In cases where the tab strip DOES divide by minWidth, we don't get the partial tab anyway.  It seems like if we want users to rely on this cue, we should ensure it's there all the time.

Pros:
* Two cues could be better than one... maybe 2I isn't enough.  And 2C still requires users to have the dropdown open rather than glancing up from their webpage.

My main opposition to it is on visual grounds.  If someone on the visual team comes up with a mockup for how "partial tabs" should look that is amazingly clean and wonderful I'd probably be swayed.  Can anyone you're hooked up with provide this?

> 1N - I think our current minWidth (2.0b1 uses 60px) is too narrow actually; we
> want to be able to hold the favicon + " - " + "A word", which by my playing
> seems to be closer to 85px or 95px. This value is likely to be highly
> contentious but, thankfully, is easy to play around with.

I think this is really a comment on 2L.  I need to reword 1N, as it's completely confusing right now.  The intent was actually to put a constraint on 2K, namely "if we have more than one strip worth of tabs, don't allow the last tab on either end to move into the strip further than into the last slot; fill the whole strip, instead of having overflow off one edge but a big gap on the other".  This is already the behavior that the B1 solution does.  I couldn't figure out a way to word it.

FWIW, I totally agree with your comment about a better minWidth; the B1 value is a quick-and-dirty "please don't get mad at our scrollbuttons" hack.

> 2A - would promote this to a P1 item, myself; favicons significantly aid in the
>  quick identification of the desired target

I only P2ed it because AFAIK Safari gets by without it, and because I knew that sspitzer already has it done anyway, so it doesn't really matter :D

> 2C - The IE7 solution highlights the currently focused tab, and as mentioned in
> comment 21, I don't think this is needed; I'd put this in the Rejected section.

Erm, when I played with IE7 today it highlighted the visible set, not just the focused tab.  Did you misspeak?  If not, highlighting the current tab is item CA.

I've gotten feedback from a lot of different people that 2C is a good way to go, enough that I'd like to at least see it in action.  We can tear it out pretty easily if it looks distracting.  My experience was:
* It makes the bold + bullet treatment on the current tab visually distinct enough (which it currently isn't), because it visually narrows the range in which the bold item will appear
* If you assume people typically open the menu to select a tab in the non-visible set (because they just use the tab strip to get at the visible set), it immediately guides your eye to the more interesting sets of tabs by eliminating a class of tabs you know you don't care about.

Do you just think it would be too distracting, or do you disagree with one or both the above points?

> 2G - we're hoping to support the creation of new windows by dragging a tab
> offscreen in Firefox 3. I'd rather see something like the scrollspeed
> accelerating significantly the longer the mouse is held off the end of the
> tabstrip, or keyboard support for responding to Accel-1/Accel-9 while dragging
> a tab

Accelerating speed (sounds like the iPod scrollwheel behavior) sounds like a good idea (probably a P3).  I'll add it.

Keyboard support for Alt+<number> while dragging sounds frightening: what does that actually mean?  Does it drop the tab there?  Does it drop the tab where it is now, and then and select that tab?  Does it jump the strip to that tab but keep dragging and scrolling?  I wouldn't know what to expect and this seems much less discoverable than "drag off strip and drop".  Can you elaborate?

I COMPLETELY agree that "drag tab out of window" should really spawn a new window.  But given that it won't in Fx2, what I proposed seems to me like the best/most consistent thing to do.  Is there an alternative behavior on "drop outside window" that you think makes more sense?

> 3B - I was on the fence in comment 21, but I think I've convinced myself that
> this setting should remain either as a pref or a hidden pref; I can't see the
> usefulness of the toggle case, and it disrupts the "scroll-to-the-end"
> operation and conflicts with the "Open in Tabs" item we have elsewhere

How does it conflict with "Open in Tabs"?  (I haven't used that item enough to know)

I really would like to expose this behavior to toggling.  I think it actually fits better there than as a pref ("Show as many tabs as possible"), and I think we shouldn't make it a hidden pref.  What about this: make it a context menu entry for the overflow menu button.  (I think a lot of per-strip settings that are not per-tab settings could appear here.  Maybe they also should NOT appear in the context menu for a tab.)

> 3G - love it; need to ensure the entire operation completes in > 0.75s

Presumably you meant "<"?  Probably the best way to ensure this is to calculate the distance to travel and then come up with a scroll speed from that, subject to some high minimum (so that bringing a barely-offscreen tab onscreen doesn't take forever).  I'll note that.

I think scrolling actually improves usability beyond just making it "feel slick"; it gives the user a much better physical sense of the tab bar's size and layout, and helps set expectations better for where to look to find other tabs, and where in the tab bar you currently are.  But the cost/benefit is probably too low to be higher than a P3, though I'd sorely love to P2 this.

> 3H - I don't think we need to bother with this; the animation should be too
> quick to actually make this useful. Or were you thinking of a cancel use case?

Cancel use case is certainly true, but to be honest, this was driven less out of an actual use for "I want to click this tab as it's going past" and more out of a "the app should always respond to me and not ignore me".  Clicking the tab bar does something at any other time, so it should do something here; given that, 3H seemed the most logical thing.  (The other option, of course, is to dim the tab bar while it's scrolling, which makes it clear it's not accepting input; but I like this better.)

> UB - I think option 0 makes the most sense here, or, if you want to get really
> fancy, you could figure out the tab index of the currently selected tab and
> make it so that the selected tab is in that same position (ie: if I'm on tab
> number 2 of 7 visible, and I select number 9 from the list, then 8-15 are shown
> with 9 selected)

I can add that as another choice, though that seems less predictable.  What's driving this question, and my preference away from 0, is a belief that people looking at a tab will frequently want to go to nearby tabs.  Setting Distance X > 0 makes this navigation much easier.  It also reduces the amount of shifting the tab bar does on every navigation, since navigating tends to move by fewer, big hops rather than a small hop on every action.  Note that the Find bar currently uses the equivalent of "0" for this value, and it's a complete disaster.

> UD - yes

OK, will add that to the list, probably a P1 since we don't want to utterly break RTL UI.  Hopefully XUL gives us most of this for free...

> RA - someone mentioned that IE7 does this; lies. The IE7 box isn't a drop-down
> menu, it's a pop-out overlay that can be pinned as a sidebar. We're talking
> about a drop-down menu here, and should work like one.

Admittedly, I did add the idea to make this a pinnable overlay as a Controversial Idea in my latest draft :D

I'll make sure the text I use is clear about what's going on.

Thanks for the detailed feedback.
Now if the visible set is highlighted, let's say with a X% opaque color, wouldn't it be consistent to highlight the current tab with the same color but 2X% opaque?

I really don't like the bullet. It also makes the menu slightly wider / eats up space that could be used for the titles.
(In reply to comment #80)
> Now if the visible set is highlighted, let's say with a X% opaque color,
> wouldn't it be consistent to highlight the current tab with the same color but
> 2X% opaque?

No, we're definitely not doing this.  Having two background colors is already iffy, three is completely out of the question.  (If you don't believe me, mock it up and you'll see it looks really odd.)
Doesn't look odd to me.
(In reply to comment #79)
> (In reply to comment #77)
> > if 15 and a half tabs can fit the tab bar, the 'half
> > tab' moves to the overflow section and 15 tabs are expanded in size
> > appropriately to exactly fit the tab bar...
> 
> This has problems with very odd-looking behavior on window resize 

[snip]

> I could probably be convinced by playing with working mockups of each and
> finding that the stretchy behavior was actually better, but short of that I'm
> very skeptical.

 
IE7 beta 3 has both of these behaviours, neither of which I find odd-looking or distracting.
(In reply to comment #82)
> Created an attachment (id=229218) [edit]
> background coloring for visible tab set and active tab
> 
> Doesn't look odd to me.

And how would it look once you hover with your mouse over that list? Getting one further background color working right with all possible settings will be difficult enough (the OSes usually just specify "normal background" and "hovered background").
(In reply to comment #85)
> And how would it look once you hover with your mouse over that list?

I don't know exactly about other OSes, but Windows XP's dark blue is pretty distinctive.

> Getting
> one further background color working right with all possible settings will be
> difficult enough (the OSes usually just specify "normal background" and
> "hovered background").

Since I would use the same color for the visual set and active tab (only different opacity), that's not an additional problem.
I have to agree with three background colors on a menu being too much. Even without considering the highlight color, it looks too messy. Also, when the current tab is already bold-faced, I think the extra background color is excessive. I think having the bullet and bold font weight are sufficient UI cues to indicate the current tab.

Talking about whether to have a second background color at all, I think an important issue is the number of visible tabs versus the total number of tabs. If someone has 25 tabs open, but only eight are visible, then the background shading is a big gain. But in the case where the user has ten tabs open with eight visible, then the shading becomes unnecessary. Maybe there is some ratio of visible to total tabs where we can activate the shading; I was thinking along the lines of 50-60%. With a threshold, I think we keep the UI clean for most people that don't have tens of tabs open at a time, but provide increased ease of use for users who do.

Finally, with respect to 3B, my opinion is that if the menu item is there, it should be a one-time setting. Specifically, it will show all tabs on the bar and then temporarily set whatever width that comes out to as minimum width. Then as tabs are closed, the minimum width increases until the original is reached. Also, whenever another tab is added, the overflow solution is used. In other words, we're always inching the minimum tab width back towards its previous setting, never away. Also, should it be disabled when all tabs are already on the screen?
With 1A (no scrollbutton) and 1M (full tabs shown), how does a user know that there are tabs hidden from the strip without opening the dropdown?

Also, with this view of "this looks like all the tabs I have", what do the keyboard shortcuts do such as a ctrl-1 to select the first tab? Should it go to the very first tab even if it's hidden or go to the first tab out of the ones shown?

If the latter, another issue when scrolling is implemented.. Pressing ctrl-[1-9] would select the tab # out of the tabs that are scrolling by or out of the tabs that would have been shown if the scrolling had finished?

More about scrolling.. Would it scroll if one ctrl-shift-tabbed from the first tab ending up at the last tab? And if the user immediately hit ctrl-tab again, the scrolling would jerk back the other direction to the first tab?


Personally I use ctrl-# a lot to jump to a specific tab that I know is in that particular position. With tab reordering built in, the two lets me keep the important tabs in easy reach.

But with the current UI as I'm understanding it, a user could leave and come back and see a set of tabs and press ctrl-2 thinking they would go to the second tab in the strip, but mysteriously get sent to some other tab that's actually the 2nd hidden tab.



Switching tabs with the keyboard can be done through ctrl-#, ctrl-(shift)-tab, ctrl-page up/down, focus on tab strip + up/down/left/right/home/end.. and on OS X cmd-alt-left/right. Showing keyboard shortcuts on the menu is an extremely good idea, but most likely only one shortcut should be shown for a particular tab at any given time.

As a suggestion, ctrl-1 and ctrl-# should always be shown where # is the number of tabs if it's <= 8. Ctrl-(shift)-tab can override the ctrl-2 to ctrl-[#-1] displayed shortcut as those intermediate numbers can be somewhat inferred. Ctrl-9 should only show up if there are 9 or more tabs and also won't be covered up by ctrl-tab shortcuts.

This scheme should work in either situations of 1 = 1st tab possibly hidden or 1 = 1st tab visible. I'm not an expert on improving UX, but just showing that picking the right shortcuts can be tricky.



(In reply to comment #25)
> Please rather use a radio bullet than a checkmark.
I agree, but just to note.. On OS X, the examples of radio buttons in the menu show up as checkmarks. Perhaps it's a bug.
> I agree, but just to note.. On OS X, the examples of radio buttons in the menu
> show up as checkmarks.

Thanks for the note.   That explains why Safari has the checkmark.  For the pinstripe theme, I think we should consider doing the check instead of the radio.

pkasting, if you agree, could add that to your next draft of your "Prioritized design for new solution" document?
> updated screen shot, popup aligned, menuitems for completely visible tabs are
grey

ben goodger wrote:  "I would also make the popup align left rather than right, i.e. topright of popup aligns to bottomright of button."

I've done that by adding 'position="after_end"' to my menupopup.

for the "grey" menu items, I'm using the screenX and the width of the tab box object and the screenX and the width of the tab strip scroll box object to determine if a tab is "completely visible".

note, a grey background for a menuitem for a completely visible tab in the all tabs popup is just a place holder color.
> With 1A (no scrollbutton) and 1M (full tabs shown), how does a user know that
> there are tabs hidden from the strip without opening the dropdown?

do pkasting's 2H and 2I solve this problem?
>  2G. If a tab is dropped offscreen, it becomes the very first/last tab (as
>      appropriate) and the strip moves to make it visible 

I don't think this is a good idea.  

Consider the "drag a tab to the desktop to create a shortcut" or "drag tabs between windows" scenarios.  

I think we should continue to do what we do now, and just create a shortcut, or load the url in the other window, and not do anything else.

These actions should not change the tab order in the source window or scroll the tab strip.
(In reply to comment #92)
> do pkasting's 2H and 2I solve this problem?

So the only way to scroll is to use the drop-down (or the keyboard) and you don't have any indication whether there are more (already read) tabs to the left/right unless you open the drop-down? Might take a while to get used to it. Without scroll buttons I'd probably have preferred to always see the first few tabs as in Safari's case. While this might be similarly awkward in the overflow case, it's at least closer to what we've already got with Firefox 1.5 and people are at least slightly used to it.
We should be working on this in a wiki doc, really, much better for collaboration

> (In reply to comment #78)
> > 1M - I actually disagree with this; partially cut off tabs add a subtle visual
> > cue that lets users know when they do have more tabs open than are currently
> > displayed  in the tabstrip - assuming 1C, it seems important to me that we have
> > this cue
> 
> My main opposition to it is on visual grounds.  If someone on the visual team
> comes up with a mockup for how "partial tabs" should look that is amazingly
> clean and wonderful I'd probably be swayed.  Can anyone you're hooked up with
> provide this?

The team working on the new theme can come up with something, but let's do the 
right thing first, and figure out aesthetics after.

> > 2A - would promote this to a P1 item, myself; favicons significantly aid in the
> >  quick identification of the desired target
> 
> I only P2ed it because AFAIK Safari gets by without it, and because I knew that
> sspitzer already has it done anyway, so it doesn't really matter :D

Just because its implemented doesn't make it a P2

> > 3B - I was on the fence in comment 21, but I think I've convinced myself that
> > this setting should remain either as a pref or a hidden pref; I can't see the
> > usefulness of the toggle case, and it disrupts the "scroll-to-the-end"
> > operation and conflicts with the "Open in Tabs" item we have elsewhere
> 
> How does it conflict with "Open in Tabs"?  (I haven't used that item enough to
> know)

Visually, I think, but I'm not sure exactly what this was about.

> I really would like to expose this behavior to toggling.  I think it actually
> fits better there than as a pref ("Show as many tabs as possible"), and I think
> we shouldn't make it a hidden pref.  What about this: make it a context menu
> entry for the overflow menu button.  (I think a lot of per-strip settings that
> are not per-tab settings could appear here.  Maybe they also should NOT appear
> in the context menu for a tab.)

I don't see a compelling case for a toggle (noting that if the toggle hides the 
menu, how can you get it back if not in the prefpanel?)  let's focus on implementing
the best way of handling overflow, and let extensions/hidden prefs handle the edge case
of people not liking the way we handle the edge case.  If the solution we're driving
in this bug is the right solution, you shouldn't be building in fallbacks in the UI


> > UB - I think option 0 makes the most sense here, or, if you want to get really
> > fancy, you could figure out the tab index of the currently selected tab and
> > make it so that the selected tab is in that same position (ie: if I'm on tab
> > number 2 of 7 visible, and I select number 9 from the list, then 8-15 are shown
> > with 9 selected)
> 
> I can add that as another choice, though that seems less predictable.  What's
> driving this question, and my preference away from 0, is a belief that people
> looking at a tab will frequently want to go to nearby tabs.  Setting Distance X
> > 0 makes this navigation much easier.  It also reduces the amount of shifting
> the tab bar does on every navigation, since navigating tends to move by fewer,
> big hops rather than a small hop on every action.  Note that the Find bar
> currently uses the equivalent of "0" for this value, and it's a complete
> disaster.

anything except 0 ends up being "clever" and that often results in UI that isn't easily understood

> > UD - yes
> 
> OK, will add that to the list, probably a P1 since we don't want to utterly
> break RTL UI.  Hopefully XUL gives us most of this for free...

luckily, it does!
Attachment #229260 - Attachment description: updated screen shot, popup aligned, menuitems for completely visible tabs are lightgrey → updated screen shot, popup aligned, menuitems for completely visible tabs are lightgrey, with mouse over that highlighted (Untitled) tab in the middle of the visible tabs.
(In reply to comment #87)
> I have to agree with three background colors on a menu being too much. Even
> without considering the highlight color, it looks too messy. Also, when the
> current tab is already bold-faced, I think the extra background color is
> excessive. I think having the bullet and bold font weight are sufficient UI
> cues to indicate the current tab.

Yep.

> Maybe there is some ratio
> of visible to total tabs where we can activate the shading; I was thinking
> along the lines of 50-60%. With a threshold, I think we keep the UI clean for
> most people that don't have tens of tabs open at a time, but provide increased
> ease of use for users who do.

I understand this point and have worried about it, but I haven't come up with a solution that's predictable for users.  Giving cues means you must make it very clear what you're hinting at and when, and I don't think this does that, which kind of scuttles it for me.  What I _do_ think we can do is not color the entries at all when there is no overflow (because there aren't two sets to distinguish), and make sure the color difference is subtle enough that it doesn't look bad to do this (so, for example, sspitzer's latest screenshot https://bugzilla.mozilla.org/attachment.cgi?id=229251 would fail this test).

> Finally, with respect to 3B, my opinion is that if the menu item is there, it
> should be a one-time setting. Specifically, it will show all tabs on the bar
> and then temporarily set whatever width that comes out to as minimum width.

Hmmmm.  I'm not sure.  I'll stick it in the controversial ideas list.

> Also, should it be disabled when all tabs are
> already on the screen?

Yes.  Good point.

(In reply to comment #88)
> With 1A (no scrollbutton) and 1M (full tabs shown), how does a user know that
> there are tabs hidden from the strip without opening the dropdown?

See comment 79.  The dropdown is glowing if there are hidden tabs they're unaware of.

You guys may be right, it may not be good enough.  But consider also the cases where the strip width divides evenly by the minwidth, or where the user is at the very end of his tab strip.  In both cases, no partial tab shows, but the strip has more tabs.  So if we use "partial tab" as a cue, it's not consistent.  How should this be solved?

Of course, taking a cue from how [shift+]ctrl+tab can wrap around, we could just always treat the set of tabs as a loop, so you show a partial first tab when you're at the last tab.  That would solve the second point above.  But it wouldn't solve the first, and it might confuse users, since it throws into question the expectations users will have for alt+<number> and for the ordering of the entries in the dropdown.

> Also, with this view of "this looks like all the tabs I have", what do the
> keyboard shortcuts do such as a ctrl-1 to select the first tab? Should it go to
> the very first tab even if it's hidden or go to the first tab out of the ones
> shown?

alt+1 always goes tot he very first tab, not the first visible tab.  alt+2 goes to the second.  alt+9 goes to the very last tab, not the last visible tab.

> More about scrolling.. Would it scroll if one ctrl-shift-tabbed from the first
> tab ending up at the last tab? And if the user immediately hit ctrl-tab again,
> the scrolling would jerk back the other direction to the first tab?

Well, if we're scrolling, then we've gotten to 3G.  So, yes, both of the above.

Assuming we don't implement 3G, then we just jump the strip, like we will for any other selection of an offscreen tab.

> But with the current UI as I'm understanding it, a user could leave and come
> back and see a set of tabs and press ctrl-2 thinking they would go to the
> second tab in the strip, but mysteriously get sent to some other tab that's
> actually the 2nd hidden tab.

I don't think users unused to Fx1.5 will expect anything from alt+<number>; I don't think they'll realize it exists, unless we show the shortcuts for it on the dropdown, in which case they'll expect whatever it is we do, because that's what their experience is.

As for Fx1.5 users, there never was an overflow case in 1.5, so I'm not convinced they'll have a ton of expectations either.  Both behaviors would probably be reasonable expectations, but having the shortcuts bound to the very first and last tabs, rather than first and last visible tabs, is far more usable.

> Switching tabs with the keyboard can be done through ctrl-#, ctrl-(shift)-tab,
> ctrl-page up/down, focus on tab strip + up/down/left/right/home/end.. and on OS
> X cmd-alt-left/right. Showing keyboard shortcuts on the menu is an extremely
> good idea, but most likely only one shortcut should be shown for a particular
> tab at any given time.

Certainly.  And we clearly wouldn't bother showing the "focus on tab strip + arrows", or probably OS X-specific shortcuts.  Like with "Find Next/Previous", which have multiple shortcut keys, we'll pick one to show in a menu, and make it a cross-platform one (then silently support platform conventions as well).  I think probably showing the numeric shortcuts, plus [shift+]ctrl+tab, is sufficient.  When they overlap, we prefer the [shift+]ctrl+tab shortcuts, because we expect people to remember and use them much more than the numeric shortcuts.

In fact, I'm inclined to not show alt+<2-8> ever, just "first", "last", "previous", and "next" shortcuts... that reduces the chance that we find the shortcuts are too much visual clutter and rip them out altogether.

> Ctrl-9 should only show up if there are 9 or more tabs and also won't be
> covered up by ctrl-tab shortcuts.

Actually, since in Fx2 the "9" shortcut should always go to the last tab (AFAIK), I'd prefer to peg it than whatever number corresponds to the number of open tabs.

(In reply to comment #90)
> > I agree, but just to note.. On OS X, the examples of radio buttons in the menu
> > show up as checkmarks.
> 
> Thanks for the note.   That explains why Safari has the checkmark.  For the
> pinstripe theme, I think we should consider doing the check instead of the
> radio.

If we did the other menus as checks instead of bullets for platform consistency's sake, and not because of a bug or that it was overlooked, then I agree.  I'll add some notes to the doc.

(In reply to comment #91)
> > updated screen shot, popup aligned, menuitems for completely visible tabs are
> grey
> 
> note, a grey background for a menuitem for a completely visible tab in the all
> tabs popup is just a place holder color.

Yes, and a fairly disturbing one... can we use the same colors as on the tabs themselves as the colors?  https://bugzilla.mozilla.org/attachment.cgi?id=229221 looks somewhat close to this.

(In reply to comment #93)
> >  2G. If a tab is dropped offscreen, it becomes the very first/last tab (as
> >      appropriate) and the strip moves to make it visible 
> 
> I don't think this is a good idea.  
> 
> Consider the "drag a tab to the desktop to create a shortcut" or "drag tabs
> between windows" scenarios.

Ah, I wasn't aware that dropping on the desktop currently created a shortcut.  OK, we shouldn't break that.  I'll change the doc.
  
> These actions should not change the tab order in the source window or scroll
> the tab strip.

So, I still think that while the user is holding the tab off-window, the strip should be scrolling.  I think that's both intuitive, and the only way to usably drag a tab to a currently-hidden part of the strip.  But if dropping doesn't move the tab, then when the user drops, the strip should either snap or scroll back to the original location.  Probably snap.  (Certainly snap if 3G isn't implemented.)

(In reply to comment #95)
> > My main opposition to it is on visual grounds.  If someone on the visual team
> > comes up with a mockup for how "partial tabs" should look that is amazingly
> > clean and wonderful I'd probably be swayed.  Can anyone you're hooked up with
> > provide this?
> 
> The team working on the new theme can come up with something, but let's do the 
> right thing first, and figure out aesthetics after.

I guess I lied that my main opposition is visual.  I'm not sure it's the right thing either, see comments above.  (I do agree with you on principle, though, and I'll modify the design doc to have more detail on this particular topic.)

> > I really would like to expose this behavior to toggling.
> 
> I don't see a compelling case for a toggle (noting that if the toggle hides the 
> menu, how can you get it back if not in the prefpanel?)

The toggle doesn't hide the menu, it just changes the effective minwidth.  We're still showing the menu in non-overflow cases.

The case is, someone temporarily needs to quickly, constantly get at a LOT of tabs.  Because they're doing research on some item's price across 20 sites, or they're me trying to bounce between all the screenshots on this bug.  So we let them temporarily trade off easier tab access for smaller, less descriptive targets.

I don't know if it's compelling enough to have the behavior, but that's why _I_ want it.  I agree that "hey, your overflow solution sucks, give me 1.5 back" is not a good enough reason to build a "fallback" here, that's not what this should be.

> > > 0 makes this navigation much easier.  It also reduces the amount of shifting
> > the tab bar does on every navigation, since navigating tends to move by fewer,
> > big hops rather than a small hop on every action.
> 
> anything except 0 ends up being "clever" and that often results in UI that
> isn't easily understood

Violent disagreement.  See the scrolling in most text editors on the down arrow key, which usually doesn't just scrolldown one line, but rather scrolls chunk-at-a-time.

We're not trying to place the tab in "the perfect" location, we should bring it onscreen to a very consistent location every single time.

(In reply to comment #96)
> Created an attachment (id=229260) [edit]
> updated screen shot, popup aligned, menuitems for completely visible tabs are
> lightgrey

Better, but I still think the difference between the visible and non-visible set may be too great.  See suggested colors above.

OK, I'm going to go off and make a lot of updates to the design doc, then post a new version.
favor: Could someone help create some artwork for me?

See
http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/winstripe/global/icons/close.png

And then see the "tab" preferences dialog icons from:

http://lxr.mozilla.org/mozilla1.8/source/browser/themes/winstripe/browser/preferences/Options.png
http://lxr.mozilla.org/mozilla1.8/source/browser/themes/pinstripe/browser/preferences/Options.png

If someone could create the series of 16x16 versions of these icons for both winstrip and pinstripe, that would really help me out.

Note, these would just be place holders, to be checked in, but replaced once we have the visual refresh for 2.0. 
(In reply to comment #97)
> (In reply to comment #87)
> > Maybe there is some ratio
> > of visible to total tabs where we can activate the shading; I was thinking
> > along the lines of 50-60%. With a threshold, I think we keep the UI clean for
> > most people that don't have tens of tabs open at a time, but provide increased
> > ease of use for users who do.
> 
> I understand this point and have worried about it, but I haven't come up with a
> solution that's predictable for users.  Giving cues means you must make it very
> clear what you're hinting at and when, and I don't think this does that, which
> kind of scuttles it for me.  What I _do_ think we can do is not color the
> entries at all when there is no overflow (because there aren't two sets to
> distinguish), and make sure the color difference is subtle enough that it
> doesn't look bad to do this (so, for example, sspitzer's latest screenshot
> https://bugzilla.mozilla.org/attachment.cgi?id=229251 would fail this test).

That's a good point. If the highlighting seems to magically appear and disappear, the clarity of the cue is lost. If the color is subtle enough, then I'd agree that just making sure it's off when all tabs are visible is sufficient.

> (In reply to comment #91)
> > > updated screen shot, popup aligned, menuitems for completely visible tabs are
> > grey
> > 
> > note, a grey background for a menuitem for a completely visible tab in the all
> > tabs popup is just a place holder color.
> 
> Yes, and a fairly disturbing one... can we use the same colors as on the tabs
> themselves as the colors?

The grays in sspitzer's most recent screen caps are not subtle enough, I agree. Maybe something along the lines of the yellow used as a background when a popup is blocked? Or maybe the standard gray with just a tinge of yellow in it, enough so it's perceptible, but not so much that it jars the senses.
Attached image Tab Preview suggestion
Mockup: How about showing a preview of tabs as the user hovers over them, especially as some sites have the same title for all their pages. Temporarily switching to it would probably be too confusing, but a picture of it could be shown over the current tab. To prevent this from slowing down the UI, you would only show it if the user hovers over a menuitem for a little while, like a tooltip. Bonus marks for translucency ;)
Presumeably only P3.

(In reply to comment #91)
> ben goodger wrote:  "I would also make the popup align left rather than right,
> i.e. topright of popup aligns to bottomright of button."
> 

I think I actually preferred the old behaviour, since that way the popup covers less of your browser window (it covers whatever app you had to the right of the window instead).
Attached image winstripe-Options.png (obsolete) —
(In reply to comment #98)
> If someone could create the series of 16x16 versions of these icons for both
> winstrip and pinstripe, that would really help me out.
> 
> Note, these would just be place holders, to be checked in, but replaced once we
> have the visual refresh for 2.0.

I don't know what it's good for, but here it is ... Downloads icon from http://lxr.mozilla.org/mozilla1.8/source/browser/themes/winstripe/browser/Toolbar-small.png -- the others are simply resized and sharpened.
Attached image pinstripe-Options.png (obsolete) —
> Mockup: How about showing a preview of tabs as the user hovers over them,
> especially as some sites have the same title for all their pages. Temporarily
> switching to it would probably be too confusing, but a picture of it could be
> shown over the current tab. To prevent this from slowing down the UI, you would
> only show it if the user hovers over a menuitem for a little while, like a
> tooltip. Bonus marks for translucency ;)
> Presumeably only P3.

I think that's pretty cool.  vlad said he might have time to work on that p3.
dao, thanks for reducing and sharpening those images for me.

What I was looking for was the images (plus two more) in a series like this:

http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/winstripe/global/icons/close.png

Could you help me out?
> What I was looking for was the images (plus two more) in a series like this:

to clarify, just the "tab" image of the large series in a series of four (regular, hovered, clicked, disabled)
(In reply to comment #105)
> > What I was looking for was the images (plus two more) in a series like this:
> 
> to clarify, just the "tab" image of the large series in a series of four
> (regular, hovered, clicked, disabled)

Yes, I can do that.
Attached image tabs.png (obsolete) —
(In reply to comment #104)
> What I was looking for was the images (plus two more) in a series like this:
> http://lxr.mozilla.org/mozilla1.8/source/toolkit/themes/winstripe/global/icons/close.png
Attachment #229286 - Attachment is obsolete: true
Attachment #229287 - Attachment is obsolete: true
Attached image tabs.png for pinstripe (obsolete) —
Attached image tabs.png for winstripe
alpha channel was missing
Attachment #229296 - Attachment is obsolete: true
Attached image tabs.png for pinstripe
Attachment #229303 - Attachment is obsolete: true
(In reply to comment #108)
> Created an attachment (id=229298) [edit]
> using -moz-Dialog for the "visible" tab menuitem background color
> 

Somehow that doesn't feel quite right - perhaps -moz-dialog is confusing because of its connotations. Maybe a very light blue color would make sense, as I believe most platforms use blue for selections, and visible tabs have been "indirectly selected".
(In reply to comment #100)
> Mockup: How about showing a preview of tabs as the user hovers over them,
> especially as some sites have the same title for all their pages. Temporarily
> switching to it would probably be too confusing, but a picture of it could be
> shown over the current tab.

Love it.  Added as P3.

> (In reply to comment #91)
> > ben goodger wrote:  "I would also make the popup align left rather than right,
> > i.e. topright of popup aligns to bottomright of button."
> 
> I think I actually preferred the old behaviour, since that way the popup covers
> less of your browser window (it covers whatever app you had to the right of the
> window instead).

I sympathize, but more important here is to be consistent with what we do elsewhere.  The dropdowns for the back/forward buttons left-align with the left edge of the button, so for this dropdown we should right-align with the right edge of the menu button.  Added that specific bit to the spec.

(In reply to comment #112)
> > Created an attachment (id=229298) [edit]
> > using -moz-Dialog for the "visible" tab menuitem background color
> 
> Somehow that doesn't feel quite right - perhaps -moz-dialog is confusing
> because of its connotations. Maybe a very light blue color would make sense, as
> I believe most platforms use blue for selections, and visible tabs have been
> "indirectly selected".

I like it enough that I'm putting it in the spec for now, even though I'm not sure it's perfect.  Perhaps a highly-desaturated version of whatever the user's selection color is would be better (or one that's been pushed very far towards our default background color).  Not sure what facilities we have in terms of doing those sorts of color transformations at runtime.  In the meantime, let's go with this until I can get one in my hands and play with it.

I've been continuing to worry about "indicating tab overflow" as beltzner and others have touched on, and I think I have a solution: we use the menu button glow for that, instead of my more-complex "tabs you haven't seen yet" indication.  Updated spec coming up shortly.
Here's the latest doc revision.

Changes:
* Clarified point 1N
* Moved RTL compliance from UQ to P1, and favicons from P2 to P1
* Note mac bullet/checkbox issue
* Spec dropdown alignment
* Spec dropdown entry background colors
* Spec which keyboard shortcuts are shown
* Changed drag-and-drop-off-window to match preexisting behavior
* Menu glows to indicate overflow
* Noted that hover should change menu appearance
* Menu button has context menu
* Killed Show All Tabs option (I've become convinced it's extension fodder)
* More details about scrolling algorithm
* Added right-click-on-dropdown-enty and tab preview lightbox suggestions
* Many controversial ideas: tab width expands to fill whole strip, reintroduce scrollbuttons, dockable dropdown (to sidebar) a la IE7, show more keyboard shortcuts, screw with Distance X in weird ways
* Many rejected ideas: looping the tab strip, multi-shaded non-visible/visible/current entries, plus some listed above
Attachment #229193 - Attachment is obsolete: true
(In reply to comment #113)
> I've been continuing to worry about "indicating tab overflow" as beltzner and
> others have touched on, and I think I have a solution: we use the menu button
> glow for that, instead of my more-complex "tabs you haven't seen yet"
> indication.  Updated spec coming up shortly.
> 

The only problem is that you still don't know whether you have tabs off to the left, off to the right, or both.

It would be clearer if we had some kind of indicator at each end of the tab bar where tabs are hidden. Perhaps deliberately placing partial tabs is the way forward? You could probably remove/reduce the need for padding too that way.

Although if you have indicators at each end then you might as well make them scroll buttons. Hum...
(In reply to comment #100)
> Mockup: How about showing a preview of tabs as the user hovers over them,
> especially as some sites have the same title for all their pages. Temporarily
> switching to it would probably be too confusing, but a picture of it could be
> shown over the current tab. To prevent this from slowing down the UI, you would
> only show it if the user hovers over a menuitem for a little while, like a
> tooltip.

Shounds pretty much like Opera 9. Note that they don't have a very useful implementation, because waiting for the thumbnail to show up takes longer than simply selecting the tab in question.

While we're at it, the previously discussed "Show All Tabs" could show similar thumbnails, see bug 308492.
> The only problem is that you still don't know whether you have tabs off to the
> left, off to the right, or both.

<snip>

> Although if you have indicators at each end then you might as well make them
> scroll buttons. Hum...

good point.  mconnor agrees, and thinks the scroll button (and all the existing scrolling functionality should come back.)

mconnor also thinks we should follow the search box ui and:

1)  selected tab is bold
2)  no checkbox, no radio
3)  no special colors

Let me attach a screen shot of what this looks like.
thanks again dao for those images.  I'm using them for winstripe and I will for pinstripe as well.

note:  my apologies, I only needed two regular and hover, sorry about doubling your work like that.
(In reply to comment #117)
> mconnor also thinks we should follow the search box ui and:
> 
> 2)  no checkbox, no radio

If anything, the search box should change to include a radio bullet!

You're welcome, Seth.

(In reply to comment #115)
> The only problem is that you still don't know whether you have tabs off to the
> left, off to the right, or both.

That's a problem if you have scroll buttons. If you only have the menu, what's the point of knowing where the tabs are off before actually opening the menu?

(In reply to comment #117)
> mconnor also thinks we should follow the search box ui and:
> 3)  no special colors

The search box doesn't bring along the same semantics, i.e. "set of currently visible tabs". That's useful in favour of orientation and losing it is backwards.
(In reply to comment #115)
> It would be clearer if we had some kind of indicator at each end of the tab bar
> where tabs are hidden. Perhaps deliberately placing partial tabs is the way
> forward? You could probably remove/reduce the need for padding too that way.
> 
> Although if you have indicators at each end then you might as well make them
> scroll buttons. Hum...

You're walking down a path I've already trod, both internally and with other people; that's why I placed the "re-add scrollbuttons" item into the controversial items list.  mconnor is a proponent of this approach.  It has pros and cons.

If we were to re-add scrollbuttons, they'd need to function much better than the B1 versions did.  I'm not 100% set against them, but I think if we can create something that works without them we'll be better off.  We'll see.  First things first in getting the P1 items done, and all that.

(In reply to comment #116)
> > Mockup: How about showing a preview of tabs as the user hovers over them,
> > especially as some sites have the same title for all their pages.
> 
> Shounds pretty much like Opera 9. Note that they don't have a very useful
> implementation, because waiting for the thumbnail to show up takes longer than
> simply selecting the tab in question.

If this happens, it should be instant, not after a delay.  I think we ought to be able to do that, though; I don't see why it should take any time to pull off.

> While we're at it, the previously discussed "Show All Tabs" could show similar
> thumbnails, see bug 308492.

Again, this is extension fodder in my view.  Possibly _good_ extension fodder, but extension fodder nonetheless.

(In reply to comment #119)
> Created an attachment (id=229318) [edit]
> screen shot following mconnor's ui suggestions

Apologies to mconnor, but I think this looks pretty bad, and certainly less distinct than even the version with just a bullet and no coloring.  I could be convinced that the coloring is a bad idea; I'm pretty dead set on the bullet, though.

(In reply to comment #121)
> > mconnor also thinks we should follow the search box ui and:
> > 
> > 2)  no checkbox, no radio
> 
> If anything, the search box should change to include a radio bullet!

I agree, though given that the search box doesn't usually have many choices, it's really more of a consistency than a necessity thing.
(In reply to comment #123)
> If this happens, it should be instant, not after a delay.  I think we ought to
> be able to do that, though; I don't see why it should take any time to pull
> off.

Because if tooltips show up instantly, skimming through the list will be all of a jitter. This could be solved by fixing the thumbnails to certain position to show up, e.g. at the bottom of the menu.
(In reply to comment #124)
> > If this happens, it should be instant, not after a delay.  I think we ought to
> > be able to do that, though; I don't see why it should take any time to pull
> > off.
> 
> Because if tooltips show up instantly, skimming through the list will be all of
> a jitter. This could be solved by fixing the thumbnails to certain position to
> show up, e.g. at the bottom of the menu.

That's not exactly what "I don't see why..." meant; I meant I saw no technical reason.

The plan, if this was done would be to put it in a lightbox in the center of the screen, which does indeed fix the position.  We'd need to worry about small window + big menu combos.

Of course, if it was in the center of the content area, we could do a snazzy zoom-out-to-the-whole-page-size effect when the user actually clicked on it.  (One that took like 0.2s, of course.)  Now THAT might be technically difficult, but I'm always on the lookout for Apple-esque UI polish bits.
Attached patch patch (obsolete) — Splinter Review
note, this patch includes some crude animation to flash the "all tabs" menu box orange 3 times when a new tab is opened in the background and it is not completely visible.  this issue is bug #342900.
Attachment #229336 - Flags: review?(mconnor)
Comment on attachment 229336 [details] [diff] [review]
patch

this patch matches https://bugzilla.mozilla.org/attachment.cgi?id=229318

seeking additional review from asaf.
Attachment #229336 - Flags: review?(bugs.mano)
Attached patch patch (obsolete) — Splinter Review
Attachment #229336 - Attachment is obsolete: true
Attachment #229338 - Flags: review?(bugs.mano)
Attachment #229336 - Flags: review?(mconnor)
Attachment #229336 - Flags: review?(bugs.mano)
Attachment #229338 - Flags: review?(mconnor)
Comment on attachment 229338 [details] [diff] [review]
patch

>Index: content/widgets/tabbrowser.xml
>+          setTimeout(this._startFlashAllTabsBox, 0, this);

Rather use

setTimeout(function(self) { self._startFlashAllTabsBox(); }, 0, this);

(or something alike) instead which gives you a correct /this/ in the methods below (which is easier to read).

>+          // IMPORTANT:  is the new tab always the last tab?

Since you're setting a timeout before getting here there's all sort of funny stuff which could happen to that tab before. You'll have to grab the tab from the TabOpen event handler and then pass it around.
Comment on attachment 229338 [details] [diff] [review]
patch

General comments:
 * I couldn't actually test this, mostly becuase you didn't include the Pinstripe changes.
 * This patch was supposed to replace the arrowscrollbox with a normal scrollbox, wasn't it? If so, please remove the click-to-scroll binding (as much as I'm still grumbling on the waste of time).
 * Any keyboard navigation solution here? Should we put the new button in the tab order?
 * You don't need second-review on toolkit/content patches (you do need ui-review).

Index: content/widgets/tabbrowser.xml
===================================================================

   <binding id="tabbrowser-tabs"
            extends="chrome://global/content/bindings/tabbox.xml#tabs">
     <content>
       <xul:arrowscrollbox anonid="arrowscrollbox" orient="horizontal" flex="1" style="min-width: 1px;" clicktoscroll="true">
         <children includes="tab"/>
       </xul:arrowscrollbox>
+      <xul:hbox class="tabs-alltabs-box" align="center" pack="end" 
+                anonid="alltabs-box">
+        <xul:toolbarbutton class="tabs-alltabs-button" type="menu">
+          <xul:menupopup anonid="alltabs-popup" position="after_end"
+            onpopupshowing="this.parentNode.parentNode.parentNode._buildAllTabsPopup()"/>

This is an anonymous node - you can probably just do |onpopupshowing="_buildAllTabsPopup();"|.


+            menuItem.setAttribute("oncommand", "this.parentNode.parentNode.parentNode.parentNode._handleAllTabsSelect(event.originalTarget)");
...
+      <method name="_handleAllTabsSelect">
+        <parameter name="aTarget"/>
+        <body><![CDATA[
+          var tabs = this.parentNode.parentNode.parentNode.mTabs;
+          this.selectedItem = tabs[aTarget.getAttribute("tabindex")];
+          this._handleTabSelect();
+        ]]></body>
+      </method>
+

This relies too heavily on the binding content structure and would make exteneding it almost impossible. Also, we shouldn't get into the point where |this| in a binding method points to anything but the binding itself. I think we should use addEventListener here, and pass a js object (should be in a field) which implements nsIDOMEventListener.

+      <method name="_handleTabOpen">
+        <body><![CDATA[
+          this.adjustTabstrip(false); 
+
+          // wait before flashing, as we might select the tab we just opened
+          // (example: "One New Tab") or open link in new tab in foreground.
+          // in cases like that, we don't want flash.
+          setTimeout(this._startFlashAllTabsBox, 0, this);

I couldn't figure why we're listening to TabFoo events, they're being dispatched from the tabbrowser binding itself, in contextes in which you know if the new tab should be selected. Add |this.mTabContainer.adjustTabstrip(false);| to |tabbrowser::addTab| and the flashing stuff to |tabbrowser::loadOneTab| (which has aLoadInBackground as an argument).


+      <field name="mFlashStart">
+        6
+      </field>
+
+      <field name="mFlashInterval">
+        150
+      </field>

ui-review ;)

+      <method name="_startFlashAllTabsBox">
...
+          // if the new tab is selected, we don't need to calculate
+          // if it is completely visible or not

Didn't you file a bug on moving this calculation to nsIScrollBoxObject?

...
+          //
+          // XXX todo
+          // IMPORTANT:  is the new tab always the last tab?

AFAICT :)

+      <method name="_flashAllTabsBox">
+        <parameter name="aTabContainer"/>
+        <body><![CDATA[
+          aTabContainer.mFlashStage--;
+
+          aTabContainer.mAllTabsBox.setAttribute("flash",
+            (aTabContainer.mFlashStage % 2) ? "true" : "false");
+
+          if (aTabContainer.mFlashStage) {
+            setTimeout(aTabContainer._flashAllTabsBox,  
+                       aTabContainer.mFlashInterval, 
+                       aTabContainer);

Use a TYPE_REPEATING_SLACK timer instead.


+      <method name="_buildAllTabsPopup">
+        <body><![CDATA[
+          // clear out the menu popup
+          var popup = this.mAllTabsPopup;
+          while (popup.hasChildNodes()) {
+            popup.removeChild(popup.lastChild);
+          }
+
+          // set up the menu popup
+          var tabbrowser = this.parentNode.parentNode.parentNode;

This relies on the tabbrowser binding content structure, please use |document.getBindingParent(this)| instead.

+          var tabs = tabbrowser.mTabs;
+
+          for (var i = 0; i < tabs.length; i++) {
+            var menuItem = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "menuitem");

nit: try not to pass the 80-characters-per-line limit.

+            var curTab = tabs[i];
+
+            menuItem.setAttribute("tabindex", i);

You don't need to set the tabindex attribute.

+            menuItem.setAttribute("selected", curTab.getAttribute("selected"));

Make this
  if (curTab.selected)
    menuItem.setAttribute("selected", "true");

+            // xxx todo
+            // what if uri, image/favicon, title change 
+            // while this popup is open?

Be careful of leaks when addressing this.

...
+            menuItem.setAttribute("oncommand", "this.parentNode.parentNode.parentNode.parentNode._handleAllTabsSelect(event.originalTarget)");

see comments on |handleAllTabsSelect|.

Index: themes/winstripe/global/browser.css
===================================================================

+.tabs-alltabs-button {
+  list-style-image: url("chrome://global/skin/icons/alltabs.png");
+  -moz-image-region: rect(0px, 16px, 16px, 0px);
+}



+.tabs-alltabs-button:hover {
+  list-style-image: url("chrome://global/skin/icons/alltabs.png");
+  -moz-image-region: rect(0px, 48px, 16px, 32px);
+}

nit: you only need the -moz-image-region rule in the :hover case.


Index: themes/winstripe/global/tabbox.css
===================================================================
...   
\ No newline at end of file


add a new line :)
Attachment #229338 - Flags: review?(bugs.mano) → review-
(In reply to comment #130)
> General comments:
>  * Any keyboard navigation solution here? Should we put the new button in the
> tab order?

I'm very leery of doing this since seemingly none of our other UI buttons are in the tab order.  Also, giving focus to the tab strip lets you arrow through it to navigate.  Is there something we can do here that won't break consistency with the rest of our UI?  (I suppose the button could have a word on it and get an accelerator, but I think that would be ugly.)

Seth, when you post patches, if possible, indicate which items on the design doc they address.  That'll make it easier to track the progression here.
FWIW, this is sec508'ish as you're adding few non-keyboard accessible features here (list of all tabs, tab preview, quick access to tabs, etc.).

asaf, thanks for the review.  

I'm working on your suggestions.  

Some responses to your general comments:

> * I couldn't actually test this, mostly becuase you didn't include the
> Pinstripe changes.

I haven't ported the changes over the theme change to pinstripe yet.

> * This patch was supposed to replace the arrowscrollbox with a normal
>scrollbox, wasn't it? If so, please remove the click-to-scroll binding (as much
> as I'm still grumbling on the waste of time).

According to mconnor, the plan is to keep the arrowscrollbox (with the click-to-scroll binding) for the 
functionality, and the visual indication when underflow / overflow happens.

> * Any keyboard navigation solution here? Should we put the new button in the
> tab order?

This is a good point.  ispiked (Adam) asked me a similar question.  I'll ask aaronl to comment.

asaf, thanks for the review.  

I'm working on your suggestions.  

Some responses to your general comments:

> * I couldn't actually test this, mostly becuase you didn't include the
> Pinstripe changes.

I haven't ported the changes over the theme change to pinstripe yet.

> * This patch was supposed to replace the arrowscrollbox with a normal
>scrollbox, wasn't it? If so, please remove the click-to-scroll binding (as much
> as I'm still grumbling on the waste of time).

According to mconnor, the plan is to keep the arrowscrollbox (with the click-to-scroll binding) for the 
functionality, and the visual indication when underflow / overflow happens.

> * Any keyboard navigation solution here? Should we put the new button in the
> tab order?

This is a good point.  ispiked (Adam) asked me a similar question.  I'll ask aaronl to comment.

(In reply to comment #123)
> (In reply to comment #115)
> > It would be clearer if we had some kind of indicator at each end of the tab bar
> > where tabs are hidden. Perhaps deliberately placing partial tabs is the way
> > forward? You could probably remove/reduce the need for padding too that way.
> > 
> > Although if you have indicators at each end then you might as well make them
> > scroll buttons. Hum...
> 
> You're walking down a path I've already trod, both internally and with other
> people; that's why I placed the "re-add scrollbuttons" item into the
> controversial items list.  mconnor is a proponent of this approach.  It has
> pros and cons.
> 
> If we were to re-add scrollbuttons, they'd need to function much better than
> the B1 versions did.  I'm not 100% set against them, but I think if we can
> create something that works without them we'll be better off.  We'll see. 
> First things first in getting the P1 items done, and all that.

We can invent something else, but I'm not convinced it'll be discoverable or obvious.  If something can be scrolled, it should indicate that it can be scrolled.  As has been pointed out elsewhere, scrollwheel on hover is pretty cool, but I'd never think of it if there wasn't an indication that the area could be scrolled.

What's wrong with the functionality in b1, aside from not scaling well?  Please provide details.

> (In reply to comment #116)
> > > Mockup: How about showing a preview of tabs as the user hovers over them,
> > > especially as some sites have the same title for all their pages.
> > 
> > Shounds pretty much like Opera 9. Note that they don't have a very useful
> > implementation, because waiting for the thumbnail to show up takes longer than
> > simply selecting the tab in question.
> 
> If this happens, it should be instant, not after a delay.  I think we ought to
> be able to do that, though; I don't see why it should take any time to pull
> off.


Instant means constant thumbnailing and caching of images, which is not cheap (see the extensions that already support tab preview bits and memory/CPU usage there.

> > While we're at it, the previously discussed "Show All Tabs" could show similar
> > thumbnails, see bug 308492.
> 
> Again, this is extension fodder in my view.  Possibly _good_ extension fodder,
> but extension fodder nonetheless.


if the feature was as good as I have it designed (mostly in my head) it'd be a great feature, but the footprint hurts a lot, and the CPU overhead.

> (In reply to comment #119)
> > Created an attachment (id=229318) [edit]
> > screen shot following mconnor's ui suggestions
> 
> Apologies to mconnor, but I think this looks pretty bad, and certainly less
> distinct than even the version with just a bullet and no coloring.  I could be
> convinced that the coloring is a bad idea; I'm pretty dead set on the bullet,
> though.


Why dead set on the bullet?  Using up 20 pixels or so of width to provide a secondary indication of selection (in addition to bold) seems rather pointless.  It looks like crap with the bullet, since there's leading whitespace on all but one line.

It isn't as distinct, but I don't think it needs to be, and I'd like to hear your rationale as to why this needs to be more distinct.  Going back to task-based design, the task is not to locate the current tab, but to select another tab, and we only need a subtle indication as to what tab you're already on so you don't select it again.  Calling more attention to it adds nothing to the task at hand.

Highlighting the current set, as I said elsewhere, is important only if we're overflowing at a tab width where users can clearly tell that the visble tabs are not what they're looking for.  The current proposal of 90 doesn't really do that well enough, so I don't think there's nearly as big of a gain as people are asserting here.  The task is "select a new tab" as opposed to "select a tab that isn't on screen" which is why we're listing all tabs, instead of just offscreen tabs.  If we're listing all tabs, we're assuming users will use this to switch tabs, even to those onscreen already, and we shouldn't somehow push their eyes away from any of the tabs, which is the gain from the different coloring.  It just adds visual and mental complexity to what should be a simple top to bottom scan.
(In reply to comment #135)
> We can invent something else, but I'm not convinced it'll be discoverable or
> obvious.  If something can be scrolled, it should indicate that it can be
> scrolled.  As has been pointed out elsewhere, scrollwheel on hover is pretty
> cool, but I'd never think of it if there wasn't an indication that the area
> could be scrolled.

I'm not sure I typically _want_ people to manually scroll the strip.  I don't think that's a very great way to get at tabs, and the fact that the strip itself will appear to scroll when someone chooses a different tab on it (assuming we do 3G) isn't the same, to me, as saying the strip is scrollable.

> What's wrong with the functionality in b1, aside from not scaling well?  Please
> provide details.

* It looks horrifically ugly
* The button on left interferes with the normal placement of the leftmost tab
* The buttons are too small to easily target and therefore barely aid scrolling at all
* The scroll speeds on click/hover/hold feel all wrong
* The buttons don't vanish when you reach the end of the strip, which feels totally inconsistent to me since they DO vanish when there's no overflow

So, basically, everything, which is much of why I'm so leery of scrollbuttons.

Every tab strip I've ever used that can be scrolled with buttons (most in the context of prefs dialogs that have too many tabs) has felt completely and utterly unusable.  B1's solution was no exception, and I think the violence of the community complaints before we droped the minWidth to 60 backs me up on that.

> > If this happens, it should be instant, not after a delay.  I think we ought to
> > be able to do that, though; I don't see why it should take any time to pull
> > off.
> 
> Instant means constant thumbnailing and caching of images, which is not cheap
> (see the extensions that already support tab preview bits and memory/CPU usage
> there.

I thought we already had what we needed here due to session restore etc., but it sounds like that was wishful thinking.  Still, we can usually switch from one tab to another *very* fast, and we only have to show one tab at a time, not preview all of them.  I don't think we need to cache everything in the world; I suspect we could still get something up within like 100 ms.  But again, that's just wild guessing on my part and I could be totally wrong.  SO let me backtrack and say that if this can't be both (1) fast and (2) cheap, then we shouldn't do it.  (And it's still a P3 in any case.)

> > Apologies to mconnor, but I think this looks pretty bad, and certainly less
> > distinct than even the version with just a bullet and no coloring.  I could be
> > convinced that the coloring is a bad idea; I'm pretty dead set on the bullet,
> > though.
> 
> 
> Why dead set on the bullet?  Using up 20 pixels or so of width to provide a
> secondary indication of selection (in addition to bold) seems rather pointless.
>  It looks like crap with the bullet, since there's leading whitespace on all
> but one line.

I disagree.  I think that looks far more readable and more distinct from the background content, and the leading whitespace doesn't bother me at all.

> It isn't as distinct, but I don't think it needs to be, and I'd like to hear
> your rationale as to why this needs to be more distinct.  Going back to
> task-based design, the task is not to locate the current tab, but to select
> another tab, and we only need a subtle indication as to what tab you're already
> on so you don't select it again.  Calling more attention to it adds nothing to
> the task at hand.

The thinking is that people who have some idea of whether they're moving to a "later" or "prior" tab can use this indicator to chop off half the list.  I'm not choosing "any tab but the current one", but rather "something somewhere after this".

> Highlighting the current set, as I said elsewhere, is important only if we're
> overflowing at a tab width where users can clearly tell that the visble tabs
> are not what they're looking for.  The current proposal of 90 doesn't really do
> that well enough, so I don't think there's nearly as big of a gain as people
> are asserting here.  The task is "select a new tab" as opposed to "select a tab
> that isn't on screen" which is why we're listing all tabs, instead of just
> offscreen tabs.  If we're listing all tabs, we're assuming users will use this
> to switch tabs, even to those onscreen already, and we shouldn't somehow push
> their eyes away from any of the tabs, which is the gain from the different
> coloring.  It just adds visual and mental complexity to what should be a simple
> top to bottom scan.

If this is just a longer list that people have to top-to-bottom scan, then I think we lose.  We're still forcing visual navigation of the whole list, which is difficult with long lists of items; we're just showing less on the screen at once.  The goal of both this and the "Distance X > 0" argument is to try and limit the fraction of total tabs people need to process to find what they're looking for.

Also I'm perfectly willing to push minWidth > 90 if that isn't enough to identify tabs, though I'm not quite as skeptical as you that that's the case.

Also, the main reason I wish to list all tabs is, like making the current tab visually distinct, to keep the strip seeming unified in construction/placement and obviate the need for dual dropdowns, one at each end.  Allowing people to select a tab in the current visible set is a minor side benefit that makes it pointless to remove the dropdown in non-overflow cases, that's all.  The goal should be to make selecting the tabs people need possible with the visible strip in as many cases as we can.
How about giving up some tab strip space to help indicate there are tabs hidden on either side as well as how many tabs are hidden?

<check attachment>

I started this idea thinking that the stacked tabs at the edge of the tab strip would change in size to indicate relatively how many tabs are hidden on that edge, but a variable size button would be bad UI especially if it can shrink to be very small.

Instead, to show relatively how many tabs are hidden away, the some number of favicon of the closest-to-current-view tabs are shown. The number of favicon tabs shown could be relative to each other with a total of 6, so one side showing 4 favicon and other edge showing 2 could mean one side has twice as many hidden tabs as the other. (Some fixed max # of favicons to prevent it from stealing space from the actual tabs on the strip.)

The stacked tabs image I have is 23px wide including the 2px to the edge of the window - this is about the same size as the current overflow arrows. Clicking it would list all tabs (not just the ones hidden on that side) just like the current discussed tabs menu. However, the image is slightly misleading because there could be just 1 tab hidden on a side and it would still show a stacked tabs image.. Clicking the 24px favicon tabs would have them appear like normal tabs causing the tab strip to readjust which tabs are showing favicons/hidden/etc.

The logic for 1M (complete tabs) and 1N (as many tabs on strip as possible) would still apply, except now the tab strip is a bit smaller because it might need to share the space with "tab-strip-before" and/or "tab-strip-after".

What would be kinda neat relating to 3G would be the tab could slide down into the favicon or expand from the favicons.. :)

[Yes, I realize this breaks some requirements pkasting has set, but hopefully someone might be able to build from this idea of helping indicate tabs are hidden.]
Note: There are long-stading issues with drag & drop inside and from menus on Mac and Linux, see http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/content/bookmarksMenu.js#543 for example.
(In reply to comment #137)
> Created an attachment (id=229422) [edit]
> screen shot, favicons to hint tabs are hidden
> 
> How about giving up some tab strip space to help indicate there are tabs hidden
> on either side as well as how many tabs are hidden?

Thanks for the nicely put together mockup, but this has a lot of usability and predictability problems, so I don't think we can use it.
One of the (many) options in one of the tab extensions I used to use was to have two square scrollbuttons at the right-hand side of the tab bar, to the left of the tab bar close button.
I think this solves a few of the problems in comment 136, though I'm not sure how many new ones it creates...
(In reply to comment #132)
> FWIW, this is sec508'ish as you're adding few non-keyboard accessible features
> here (list of all tabs, tab preview, quick access to tabs, etc.).

I disagree.  The new tabs -- just like the old tabs -- are fully keyboard navigable.  You can tab to the tab bar itself and use the left and right arrow keys to select different tabs.  They even scroll into view as you arrow around!  You can see each tab's title and favicon once you select it from the tabbar.  A list of all tabs in a menu (*cough* Window menu *cough*) would be a "nice-to-have", but it is not a section 508 issue.
Not sec508 as Mark said. Might be nice if places/history would put currently open items at the top of the list so you can use quick search to look for something you already have open. Useful when you have lots of tabs open.
Attachment #229338 - Flags: review?(mconnor)
Attachment #229338 - Attachment is obsolete: true
Attachment #229495 - Flags: review?(bugs.mano)
thanks asaf for the review.  I've just attached a new patch, and here are some comments in response to your review (and my new patch.)

1)

> * Any keyboard navigation solution here? Should we put the new button in the
> tab order?

thanks again for pointing this out.

I've pinged aaronl for some ideas.  he's pointed me to mark pilgrim, who commented on this bug (see comment #141.)

2)
        
> onpopupshowing="this.parentNode.parentNode.parentNode._buildAllTabsPopup()"/>
>
> This is an anonymous node - you can probably just do
> |onpopupshowing="_buildAllTabsPopup();"|.

fixed, thanks.

3)

>+            menuItem.setAttribute("oncommand",
>"this.parentNode.parentNode.parentNode.parentNode._handleAllTabsSelect(event.originalTarget)");
>...
>+      <method name="_handleAllTabsSelect">
>+        <parameter name="aTarget"/>
>+        <body><![CDATA[
>+          var tabs = this.parentNode.parentNode.parentNode.mTabs;
>+          this.selectedItem = tabs[aTarget.getAttribute("tabindex")];
>+          this._handleTabSelect();
>+        ]]></body>
>+      </method>
>+
>
> This relies too heavily on the binding content structure and would make
> exteneding it almost impossible. 

Ah, I see what you mean.  I've switched to using |document.getBindingParent(this)| instead, as you suggested.

> Also, we shouldn't get into the point where
>|this| in a binding method points to anything but the binding itself. 

I don't see how "this" (in my original _handleAllTabsSelect() method) points to something other than the binding.  Can you elaborate?

> I think we should use addEventListener here, and pass a js object (should be in a
>field) which implements nsIDOMEventListener.

fixed, thanks.

I also fixed this in my _allTabsMenuItemCommandHandler() that replaces _handleAllTabsSelect():

> +          this.selectedItem = tabs[aTarget.getAttribute("tabindex")];
> +          this._handleTabSelect();

I didn't need that call to _handleTabSelect().  Setting the selectedItem is all I need to do.

Now I just have:

  this.tabcontainer.selectedItem = tabs[aEvent.target.getAttribute("tabstripindex")];

> I couldn't figure why we're listening to TabFoo events, they're being
> dispatched from the tabbrowser binding itself, in contextes in which you know
> if the new tab should be selected. Add
> |this.mTabContainer.adjustTabstrip(false);| to |tabbrowser::addTab| 

I've removed my _handleTabOpen() method and restored this code:

+ <handler event="TabOpen"   action="this.adjustTabstrip(false);"/>

Are you saying we don't need that handler at all?  

> and the flashing stuff to |tabbrowser::loadOneTab| (which has aLoadInBackground as an
> argument).

fixed, thanks.

> +      <field name="mFlashStart">
> +        6
> +      </field>
> +
> +      <field name="mFlashInterval">
> +        150
> +      </field>
>
>ui-review ;)

mconnor suggests a pulse / false animation, like wordpress uses when you publish.  Once I get this patch looking good, I'll work on implementing the wordpress animation.

> +      <method name="_startFlashAllTabsBox">
> ...
> +          // if the new tab is selected, we don't need to calculate
> +          // if it is completely visible or not
>
> Didn't you file a bug on moving this calculation to nsIScrollBoxObject?

I plan on doing is discussing that layout change with roc / dbaron, and then once implemented, use that here.  Note, the suggested implementation in bug #344594 would be the general case, where I'm (currently) doing something specifc.

> +          //
> +          // XXX todo
> +          // IMPORTANT:  is the new tab always the last tab?
>
> AFAICT :)

Thanks.  I thought so, too.  I've removed my comment.

> +      <method name="_flashAllTabsBox">
> +        <parameter name="aTabContainer"/>
> +        <body><![CDATA[
> +          aTabContainer.mFlashStage--;
> +
> +          aTabContainer.mAllTabsBox.setAttribute("flash",
> +            (aTabContainer.mFlashStage % 2) ? "true" : "false");
> +
> +          if (aTabContainer.mFlashStage) {
> +            setTimeout(aTabContainer._flashAllTabsBox,  
> +                       aTabContainer.mFlashInterval, 
> +                       aTabContainer);
>
> Use a TYPE_REPEATING_SLACK timer instead.

fixed, thanks.

notice, as I did with the scrollbox.xml and preferences.xml examples, I have the 

  if (!document)
    aTimer.cancel();

This issue is covered by #343586 (and I'll add this new tabbrowser.xml usage to that bug.)

>+          // set up the menu popup
>+          var tabbrowser = this.parentNode.parentNode.parentNode;
>
> This relies on the tabbrowser binding content structure, please use
> |document.getBindingParent(this)| instead.

fixed, thanks.

> +            var menuItem =
> document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul",
>"menuitem");
>
>nit: try not to pass the 80-characters-per-line limit.

fixed, thanks.  (And I fixed a couple other places in tabbrowser.xml)

>+            var curTab = tabs[i];
>+
>+            menuItem.setAttribute("tabindex", i);
>
>You don't need to set the tabindex attribute.

Whoops, I wasn't attempting to set the menuitem tabindex attribute (as in keyboard tab ordering index)
(see http://xulplanet.com/references/elemref/ref_menuitem.html#attr_tabindex)

I was setting an attribute to be used in the _handleAllTabsSelect() method to figure out which tab to make the 
selected item.  

I still need this, so I've switched to calling it the "tabstripindex".

Or, did I misunderstand your review comment?  Are you suggesting I figure out the index of the menuitem by looking at the childnodes of the popup?

>+            menuItem.setAttribute("selected",
>curTab.getAttribute("selected"));
>
>Make this
>  if (curTab.selected)
>    menuItem.setAttribute("selected", "true");

fixed, thanks.

>+            // xxx todo
>+            // what if uri, image/favicon, title change 
>+            // while this popup is open?
>
>Be careful of leaks when addressing this.

Thanks for the words of warning.
I've added your warning to the "XXX todo" comment.  
I'll be logging a spin off bug on this issue when this patch lands (for addressing this), and I'll repeat your warning there.

> +            menuItem.setAttribute("oncommand",
> "this.parentNode.parentNode.parentNode.parentNode._handleAllTabsSelect(event.originalTarget)");
>
> see comments on |handleAllTabsSelect|.

fixed, thanks.

Following your suggestion, I'm now doing:

 menuItem.addEventListener("command", this._allTabsMenuItemCommandHandler, false);

I've also added an onpopuphiding to remove the event listeners from the menu items.

> nit: you only need the -moz-image-region rule in the :hover case.

It appears that I do need it, otherwise the entire image (all four versions of the button) show up.

> Index: themes/winstripe/global/tabbox.css
> \ No newline at end of file
> add a new line :)

That message comes from the previous version, which did not have one.

thanks again asaf for the review!
> > +          //
> > +          // XXX todo
> > +          // IMPORTANT:  is the new tab always the last tab?
> >
> > AFAICT :)
> 
> Thanks.  I thought so, too.  I've removed my comment.
> 
That's a bad assumption to make, makes the life of tab extension authors harder. You ought to pass the value of event.originalTarget from the TabOpen handler through the intermediate handler function (why do you need that at all?) to _setUpFlashTimer.
(In reply to comment #144)


> I don't see how "this" (in my original _handleAllTabsSelect() method) points to
> something other than the binding.  Can you elaborate?

When you're passing a callback method to methods like addEventListener and setTimeout, "this" doesn't point to the js object in which the callback function is scoped in (when the method is called).

> > I couldn't figure why we're listening to TabFoo events, they're being
> > dispatched from the tabbrowser binding itself, in contextes in which you know
> > if the new tab should be selected. Add
> > |this.mTabContainer.adjustTabstrip(false);| to |tabbrowser::addTab| 
> 
> I've removed my _handleTabOpen() method and restored this code:
> 
> + <handler event="TabOpen"   action="this.adjustTabstrip(false);"/>
> 
> Are you saying we don't need that handler at all?

I think we should remove it, you can just call this.mTabContainer.adjustTabstrip when you're adding the tab. The event is for non-internal usage.


> > +      <method name="_startFlashAllTabsBox">
> > ...
> > +          // if the new tab is selected, we don't need to calculate
> > +          // if it is completely visible or not
> >
> > Didn't you file a bug on moving this calculation to nsIScrollBoxObject?
> 
> I plan on doing is discussing that layout change with roc / dbaron, and then
> once implemented, use that here.  Note, the suggested implementation in bug
> #344594 would be the general case, where I'm (currently) doing something
> specifc.
> 

Kk.

> > +          //
> > +          // XXX todo
> > +          // IMPORTANT:  is the new tab always the last tab?
> >
> > AFAICT :)
> 
> Thanks.  I thought so, too.  I've removed my comment.
>

So, comment 145 is right, not so important though (extension which overrides addTab can override this too).


> Whoops, I wasn't attempting to set the menuitem tabindex attribute (as in
> keyboard tab ordering index)
> (see http://xulplanet.com/references/elemref/ref_menuitem.html#attr_tabindex)
> 
> I was setting an attribute to be used in the _handleAllTabsSelect() method to
> figure out which tab to make the 
> selected item.  
> 
> I still need this, so I've switched to calling it the "tabstripindex".
> 
> Or, did I misunderstand your review comment?  Are you suggesting I figure out
> the index of the menuitem by looking at the childnodes of the popup?
>

You did understand.

> > nit: you only need the -moz-image-region rule in the :hover case.
> 
> It appears that I do need it, otherwise the entire image (all four versions of
> the button) show up.
>

Er, are you sure this wasn't a syntax error? Could you paste the style rules which lead to this issue. 
(In reply to comment #146)
> So, comment 145 is right, not so important though (extension which overrides
> addTab can override this too).

Note that this doesn't only affect extensions if you indeed set a timeout before checking for the tab's visibility. There are other consumers of addTab (e.g. tabbrowser's loadTabs and SessionStore's restoreWindow and undoCloseTab) which either add more than one tab at once or move the tab around right after adding it.

Index: content/widgets/tabbrowser.xml
===================================================================
+            this.mAllTabsBox.setAttribute("flash",
+              (this.mFlashStage % 2) ? "true" : "false");

Boolean attributes are usually not set to "false" but directly removed.

BTW: As I read this code, you flash the button three times and then leave it in flashed state. While I agree with this (as in WinXP the users sees whether there are still any unread tabs), I miss the code for removing the "flash" attribute once all newly added tabs have been visible.

+            menuItem.addEventListener("command", 
+              this._allTabsMenuItemCommandHandler, false);

What's wrong with

var self = this;
menuItem.addEventListener("command", function() { this._allTabsMenuItemCommandHandler(); }, false);

which ensures that |this| references the correct object in _allTabsMenuItemCommandHandler?
Attachment #229495 - Attachment is obsolete: true
Attachment #229543 - Flags: review?(bugs.mano)
Attachment #229495 - Flags: review?(bugs.mano)
asaf (and others) thanks for the reviews.  here's some comments about my latest patch:

>> > I couldn't figure why we're listening to TabFoo events, they're being
>> > dispatched from the tabbrowser binding itself, in contextes in which you know
>> > if the new tab should be selected. Add
>> > |this.mTabContainer.adjustTabstrip(false);| to |tabbrowser::addTab| 
>> 
>> I've removed my _handleTabOpen() method and restored this code:
>> 
>> + <handler event="TabOpen"   action="this.adjustTabstrip(false);"/>
>> 
>> Are you saying we don't need that handler at all?
>
>I think we should remove it, you can just call
>this.mTabContainer.adjustTabstrip when you're adding the tab. The event is for
>non-internal usage.

fixed, thanks.

>> > +          //
>> > +          // XXX todo
>> > +          // IMPORTANT:  is the new tab always the last tab?
>> >
>> > AFAICT :)
>> 
>> Thanks.  I thought so, too.  I've removed my comment.
>
>
>So, comment 145 is right, not so important though (extension which overrides
>addTab can override this too).

I've fixed it so that loadOneTab() calls _setUpFlashTimer() and I pass through the new tab.  I only call setUpFlashTimer() when the new tab is loaded in the background, and it is not selected, so I was able to remove some code that was checking if the new tab was selected.  Additionally, since I'm passing the new tab in, I can just get the boxObject for the new tab.

>> Or, did I misunderstand your review comment?  Are you suggesting I figure out
>> the index of the menuitem by looking at the childnodes of the popup?
>>
>
>You did understand.

fixed, thanks.

>> > nit: you only need the -moz-image-region rule in the :hover case.
>> 
>> It appears that I do need it, otherwise the entire image (all four versions of
>> the button) show up.
>
>
>Er, are you sure this wasn't a syntax error? Could you paste the style rules
>which lead to this issue. 

If I just do:

.tabs-alltabs-button {
  list-style-image: url("chrome://global/skin/icons/alltabs.png");
}

I'll see all for button images.

So I'm doing:

.tabs-alltabs-button {
  list-style-image: url("chrome://global/skin/icons/alltabs.png");
  -moz-image-region: rect(0px, 16px, 16px, 0px);
}

see also from browser.css:

.tab-icon {
  margin-top: 1px;
  -moz-margin-end: 3px;
  width: 16px;
  height: 16px;
  list-style-image: url("chrome://global/skin/icons/folder-item.png");
  -moz-image-region: rect(0px, 16px, 16px, 0px);
}

Perhaps not specifying the -moz-image-region would work if alltabs.png was a column of images, instead of a row of images?

(For winstripe, alltabs.png is this image https://bugzilla.mozilla.org/attachment.cgi?id=229305)

can you review again?  in parallel, I'm going to go port this to pinstripe.
(In reply to comment #111)
> Created an attachment (id=229307) [edit]
> tabs.png for pinstripe

I know it sounds bad, but those look like little miniature washers and dryers than tabs. Sorry.
> Perhaps not specifying the -moz-image-region would work if alltabs.png was a
> column of images, instead of a row of images?

or perhaps, I forgot my thinking cap.

Asaf pointed out that he meant for me to do this:

.tabs-alltabs-button {
  list-style-image: url("chrome://global/skin/icons/alltabs.png");
  -moz-image-region: rect(0px, 16px, 16px, 0px);
}

.tabs-alltabs-button:hover {
  -moz-image-region: rect(0px, 48px, 16px, 32px);
}

Which works fine.  new patch coming, with that change, and with pinstripe.
(In reply to comment #150)
> (In reply to comment #111)
> > Created an attachment (id=229307) [edit]
> > tabs.png for pinstripe
> 
> I know it sounds bad, but those look like little miniature washers and dryers
> than tabs. Sorry.

Well, I didn't even like the original icon: http://lxr.mozilla.org/mozilla1.8/source/browser/themes/pinstripe/browser/preferences/Options.png
If you can utilize it any better, go ahead. (But it may be waste of time, because it's only a place holder.)
(In reply to comment #151)
>   -moz-image-region: rect(0px, 16px, 16px, 0px);

I don't know if there is a code convention, but generally, 0 doesn't need a unit.
Attachment #229543 - Attachment is obsolete: true
Attachment #229543 - Flags: review?(bugs.mano)
Attachment #229591 - Attachment is obsolete: true
Attached patch unified diff (obsolete) — Splinter Review
Attachment #229593 - Attachment is obsolete: true
comments on my last patch:

1)

+.alltabs-item {
+  text-align: start;
+}

the menuitem text is aligned centered, otherwise.

2)

+.tabs-alltabs-button {
+  list-style-image: url("chrome://global/skin/icons/alltabs.png");
+  -moz-image-region: rect(0px, 16px, 16px, 0px); 
+}

no hover state for pinstripe (the the rest of the toolbar buttons)

3)  since I can't do script from a theme, I needed a new binding for the menupopup

+.tabs-alltabs-popup {
+  -moz-binding: url("chrome://global/content/bindings/tabbrowser.xml#tabbrowser-alltabs-popup");
+}

and

+  <binding id="tabbrowser-alltabs-popup"
+           extends="chrome://global/content/bindings/popup.xml#popup">
..
+  </binding>

4) fixed, per asaf:

+.tabs-alltabs-button {
+  list-style-image: url("chrome://global/skin/icons/alltabs.png");
+  -moz-image-region: rect(0px, 16px, 16px, 0px); 
+}
+
+.tabs-alltabs-button:hover {
+  -moz-image-region: rect(0px, 48px, 16px, 32px);
+}

seeking r= from asaf
Attachment #229596 - Flags: review?(bugs.mano)
(In reply to Seth Spitzer, comment 149)
>>> Or, did I misunderstand your review comment?  Are you suggesting I figure out
>>> the index of the menuitem by looking at the childnodes of the popup?
>>>

>>You did understand.

> fixed, thanks.

Er, "by you did understand" I did refer to the issue with the normal tabindex attribute, though I think this isn't a solution either way (see upcoming review).
Comment on attachment 229596 [details] [diff] [review]
fix, including winstripe change (per asaf)

Index: content/widgets/tabbrowser.xml
===================================================================
 
   <binding id="tabbrowser-tabs"
            extends="chrome://global/content/bindings/tabbox.xml#tabs">
     <content>
       <xul:arrowscrollbox anonid="arrowscrollbox" orient="horizontal" flex="1" style="min-width: 1px;" clicktoscroll="true">
         <children includes="tab"/>
       </xul:arrowscrollbox>
+      <xul:hbox class="tabs-alltabs-box" align="center" pack="end"

I'm wondering if we really need the align attribute here. If we do, please add |align="start"| attribute on the menupopup and get rid of the corresponding style rule.

+      <destructor>
+        <![CDATA[
+          // Release timers to avoid reference cycles.

nit: only one timer ;)

+      <method name="_setUpFlashTimer">

nit: maybe rename this to _notifyBackgroundTab?

+        <parameter name="aTab"/>
+        <body><![CDATA[
+          if (this.mFlashStage)
+            return;
+
+          var tsbo = this.mTabstrip.scrollBoxObject;
+          var tsboStart = tsbo.screenX;
+          var tsboEnd = tsboStart + tsbo.width;
+
+          var ctbo = aTab.boxObject;
+          var ctboStart = ctbo.screenX;
+          var ctboEnd = ctboStart + ctbo.width;
+          // only start the flash timer if the new tab (which was loaded in
+          // the background) is not completely visible
+          if (tsboStart > ctboStart || ctboEnd > tsboEnd) {

Please test RTL UI.
  
+      <method name="notify">
+        <parameter name="aTimer"/>
+        <body><![CDATA[
+          if (!document)
+            aTimer.cancel();
+
+          if (aTimer == this.mFlashTimer) {

You don't need this check - we only have one timer in this binding; if you've added this to arrowscrollbox, please fix it too while we're here.



+      <field name="_allTabsMenuItemCommandHandler" readonly="true">
+      <![CDATA[({
+        tabcontainer: this,
+        handleEvent: function handleEvent(aEvent) {
+          if (!aEvent.isTrusted) {
+            // Don't let untrusted events mess with tabs.
+            return;
+          }
+
+          var tabbrowser = document.getBindingParent(this.tabcontainer);
+          var tabs = tabbrowser.mTabs;
+
+          var menuItems = this.tabcontainer.mAllTabsPopup.childNodes;
+          for (var i = 0; i < menuItems.length; i++) {
+            if (menuItems[i] == aEvent.target) {
+              this.tabcontainer.selectedItem = tabs[i];
+              break;
+            }
+          }


See comment 160. Either way, this isn't going to work - tabs can be closed while this menu is open (|window.close()| in a popup-window opened as a tab). One way to address this is by setting the tab as a JS property on the menuitem.

+          aEvent.stopPropagation();
+          aEvent.preventDefault();

Why?

+      <method name="_onHidingAllTabsPopup">
+        <body><![CDATA[
...
+        ]]></body>
+      </method>
+
+      <method name="_onShowingAllTabsPopup">
+        <body><![CDATA[
...
+        ]]></body>
+      </method>

There is no reason not to handle the showing and hiding events inside the new binding (rather than passing them back to the tabbconatiner), the menuitems are the childNodes of this binding. Also move the command handler field into this binding.


+  <binding id="tabbrowser-alltabs-popup"
+           extends="chrome://global/content/bindings/popup.xml#popup">

Please add a comment here similar to the one above the closebutton binding.


Index: themes/pinstripe/global/browser.css
===================================================================

+/**
+ * All Tabs Button
+ */
+
+.tabs-alltabs-box[flash="true"] {
+  background-color: orange;
+}
+
+.tabs-alltabs-button {
+  list-style-image: url("chrome://global/skin/icons/alltabs.png");
+  -moz-image-region: rect(0px, 16px, 16px, 0px); 
+}
+

I think we should add a [buttondown="true"] ("pressed" state) rule here, Pinstripe has it for the back and forward buttons.

Index: themes/pinstripe/global/tabbox.css
===================================================================

+	/* alltabs */
+.alltabs-item > .menu-iconic-left > .menu-iconic-icon {
+  width: 16px;
+  height: 16px;
+  list-style-image: url("chrome://global/skin/icons/small-globe-sunken.png");
+}
+
+.alltabs-item {
+  text-align: start;
+}
+
+.alltabs-item[selected="true"] {
+  font-weight: bold;
+}
+

These rules belong to toolkit/themes/pinstripe/global/browser.css.


Index: themes/winstripe/global/tabbox.css
===================================================================

+.alltabs-item > .menu-iconic-left > .menu-iconic-icon {
+  width: 16px;
+  height: 16px;
+  list-style-image: url("chrome://global/skin/icons/folder-item.png");
+  -moz-image-region: rect(0px, 16px, 16px, 0px);
+}
+
+.alltabs-item[selected="true"] {
+  font-weight: bold;
+}

ditto (s/pinstripe/winstripe).
Attachment #229596 - Flags: review?(bugs.mano) → review-
(In reply to comment #137)
> Created an attachment (id=229422) [edit]
> screen shot, favicons to hint tabs are hidden
> 
> How about giving up some tab strip space to help indicate there are tabs hidden
> on either side as well as how many tabs are hidden?
> 
> <check attachment>
> 
> I started this idea thinking that the stacked tabs at the edge of the tab strip
> would change in size to indicate relatively how many tabs are hidden on that
> edge, but a variable size button would be bad UI especially if it can shrink to
> be very small.
> 
> Instead, to show relatively how many tabs are hidden away, the some number of
> favicon of the closest-to-current-view tabs are shown. The number of favicon
> tabs shown could be relative to each other with a total of 6, so one side
> showing 4 favicon and other edge showing 2 could mean one side has twice as
> many hidden tabs as the other. (Some fixed max # of favicons to prevent it from
> stealing space from the actual tabs on the strip.)
> 
> The stacked tabs image I have is 23px wide including the 2px to the edge of the
> window - this is about the same size as the current overflow arrows. Clicking
> it would list all tabs (not just the ones hidden on that side) just like the
> current discussed tabs menu. However, the image is slightly misleading because
> there could be just 1 tab hidden on a side and it would still show a stacked
> tabs image.. Clicking the 24px favicon tabs would have them appear like normal
> tabs causing the tab strip to readjust which tabs are showing
> favicons/hidden/etc.
> 
> The logic for 1M (complete tabs) and 1N (as many tabs on strip as possible)
> would still apply, except now the tab strip is a bit smaller because it might
> need to share the space with "tab-strip-before" and/or "tab-strip-after".
> 
> What would be kinda neat relating to 3G would be the tab could slide down into
> the favicon or expand from the favicons.. :)
> 
> [Yes, I realize this breaks some requirements pkasting has set, but hopefully
> someone might be able to build from this idea of helping indicate tabs are
> hidden.]
> 

This is similar to my idea outlined in the Google Groups discussion. I have several ideas that I think might improve this. For the hidden tab stack, it could represent the actual # of tabs until a certain point, at which it will display a number above the 'multiple tabs icon' to represent how many there are. 
Another more complex part of my original idea, was the transition. Mouse movement over the tab bar in the place of stacking would pull up previews (possibly just the favicon, or maybe the title of the page) of each of the pages. As you moved the mouse right and left, it should shift. This would allow quick access to the hidden tabs. It is important to note that this effect would only be triggered by being in the 'trigger zone,' which is essentially the area of hidden tabs, not fully displayed tabs. This assures no annoying usage of switching between tabs normally.
(In reply to comment #162)
> (In reply to comment #137)
>
> This is similar to my idea outlined in the Google Groups discussion. I have
> several ideas that I think might improve this. For the hidden tab stack, it
> could represent the actual # of tabs until a certain point, at which it will
> display a number above the 'multiple tabs icon' to represent how many there
> are. 
> Another more complex part of my original idea, was the transition. Mouse
> movement over the tab bar in the place of stacking would pull up previews
> (possibly just the favicon, or maybe the title of the page) of each of the
> pages. As you moved the mouse right and left, it should shift. This would allow
> quick access to the hidden tabs. It is important to note that this effect would
> only be triggered by being in the 'trigger zone,' which is essentially the area
> of hidden tabs, not fully displayed tabs. This assures no annoying usage of
> switching between tabs normally.
> 

This sounds a bit like the OSX dock showing more space to active programs, but still some space to others - interesting idea.
Here is how Eclipse handles tab overflow.  Things that should be noted:

1)  Number of hidden tabs is listed in the menu.
2)  Hidden tabs are in bold text.
3)  There is a search field at the top of the overflow menu to filter the list of tabs.

I would really like to see option 3 implemented in the solution being developed here.
commments on my last patch:

1)

>I'm wondering if we really need the align attribute here. If we do, please add
>|align="start"| attribute on the menupopup and get rid of the corresponding
>style rule.

chatting with asaf over irc.  we're going ot keep the align="center" 
(in both tabbrowser.xml and pinstripe/global/globalBindings.xml) but remove the corresponding style rule.
the right fix is to move the "text-align: center;" rule from pinstripe/global/browser.css from the .tabbrowser-tabs block to the .tabbrowser-tab block
so that this rule does not apply to the menuitem labels in the all-tabs popup menu.

2)

> +      <destructor>
> +        <![CDATA[
> +          // Release timers to avoid reference cycles.
>
> nit: only one timer ;)

fixed, thanks (and fixed in scrollbox.xml as well.)

3)

> +      <method name="_setUpFlashTimer">
>
> nit: maybe rename this to _notifyBackgroundTab?

fixed, thanks.

4)

>+        <parameter name="aTab"/>
>+        <body><![CDATA[
>+          if (this.mFlashStage)
>+            return;
>+
>+          var tsbo = this.mTabstrip.scrollBoxObject;
>+          var tsboStart = tsbo.screenX;
>+          var tsboEnd = tsboStart + tsbo.width;
>+
>+          var ctbo = aTab.boxObject;
>+          var ctboStart = ctbo.screenX;
>+          var ctboEnd = ctboStart + ctbo.width;
>+          // only start the flash timer if the new tab (which was loaded in
>+          // the background) is not completely visible
>+          if (tsboStart > ctboStart || ctboEnd > tsboEnd) {
>
>Please test RTL UI.

RTL works, but I found a problem with RTL.  
when I open a link in the background, we scroll (and we don't do this with LTR).
I see this in 2.0b1, so it is not my change.
I'm testing RTL by doing ":root { direction : rtl }" in my userChrome.css.
I'll log a bug on this, if there isn't one already.

5)

>+      <method name="notify">
>+        <parameter name="aTimer"/>
>+        <body><![CDATA[
>+          if (!document)
>+            aTimer.cancel();
>+
>+          if (aTimer == this.mFlashTimer) {
>
>You don't need this check - we only have one timer in this binding; if you've
>added this to arrowscrollbox, please fix it too while we're here.

fixed, thanks (and fixed in scrollbox.xml as well.)

6)

> See comment 160. Either way, this isn't going to work - tabs can be closed
> while this menu is open (|window.close()| in a popup-window opened as a tab).
> One way to address this is by setting the tab as a JS property on the menuitem.

fixed, thanks.

now I'm setting the a "tab" JS property on the menuitem and then looking
at all the tabs in handleEvent() to find a match, and if I find one, I set it as the selectedItem.

7)

>+          aEvent.stopPropagation();
>+          aEvent.preventDefault();
>
>Why?

copy-and-paste error on my part.  fixed, thanks.

> There is no reason not to handle the showing and hiding events inside the new
> binding (rather than passing them back to the tabbconatiner), the menuitems are
> the childNodes of this binding. Also move the command handler field into this
> binding.

fixed, thanks.

> +  <binding id="tabbrowser-alltabs-popup"
> +           extends="chrome://global/content/bindings/popup.xml#popup">
>
> Please add a comment here similar to the one above the closebutton binding.

fixed, thanks.

> I think we should add a [buttondown="true"] ("pressed" state) rule here,
> Pinstripe has it for the back and forward buttons.

not fixed yet, I'm still trying to get this to work.

> These rules belong to toolkit/themes/pinstripe/global/browser.css.

fixed, thanks.

> ditto (s/pinstripe/winstripe).

fixed, thanks.
here are the open issues:

1)

asaf wrote over irc:  "btw, there's missing semicolon in the command handler field"

2)

>Please test RTL UI.

RTL works, but I found a problem with RTL.  
when I open a link in the background, we scroll (and we don't do this with LTR).
I see this in 2.0b1, so it is not my change.
I'm testing RTL by doing ":root { direction : rtl }" in my userChrome.css.
I'll log a bug on this, if there isn't one alreaday.

3)

> I think we should add a [buttondown="true"] ("pressed" state) rule here,
> Pinstripe has it for the back and forward buttons.

not fixed yet, I'm still trying to get this to work.

this list excludes:

a) do mconnor's suggested animation
b) make sure the visual refresh team is aware of alltabs.png
c) go through pkastings design doc to indicate what I've done (and what I haven't.)
Comment on attachment 229754 [details] [diff] [review]
updates patch, per asaf's review, pinstripe and winstripe (three open issues)

seeking asaf's review (while work on the buttondown css rules for pinstripe / winstripe)
Attachment #229754 - Flags: review?(bugs.mano)
Whiteboard: [mustfix]
Attachment #229754 - Attachment is obsolete: true
Attachment #229782 - Flags: review?(bugs.mano)
Attachment #229754 - Flags: review?(bugs.mano)
changes in that last patch:

1) s/})]]>/});]]> (per mano, I fixed three places)

2) "tabcontainer.mAllTabsPopup == this" simplification

3) open="true" style for pinstripe

4) "tabcontainer.selectedItem = aEvent.target.tab;" simplification (and comment)

5) following the mOuter convention
> 1) s/})]]>/});]]> (per mano, I fixed three places)

asaf changed his mind about the ";" (and he said the usage in richlistbox.xml is wrong), so I won't be do that.

Comment on attachment 229782 [details] [diff] [review]
updated, per conversation with asaf over irc

Please make sure the following follow ups are filed:
  1. "Live Update" of the menuitems' label and image attributes.
  2. Removal of the corresponding menuitem of a tab if it's closed while the menu is open.
  3. Tooltip for the new button.

Index: content/widgets/tabbrowser.xml
===================================================================

...
+      <xul:hbox class="tabs-alltabs-box" align="center" pack="end" 
+                anonid="alltabs-box">
+        <xul:toolbarbutton class="tabs-alltabs-button" type="menu">
+          <xul:menupopup class="tabs-alltabs-popup"
+            anonid="alltabs-popup" position="after_end"/>


nit: make this:
+          <xul:menupopup class="tabs-alltabs-popup"
+                         anonid="alltabs-popup"
+                         position="after_end"/>

...
-          pb2.addObserver("browser.tabs.closeButtons", this._prefObserver, true);
+          pb2.addObserver("browser.tabs.disableBackgroundClose", 
+            this._prefObserver, true);
+          pb2.addObserver("browser.tabs.closeButtons", 
+            this._prefObserver, true);
 
nit: make this:
+          pb2.addObserver("browser.tabs.disableBackgroundClose", 
+                          this._prefObserver, true);
+          pb2.addObserver("browser.tabs.closeButtons", 
+                           this._prefObserver, true);



...
+  <!-- alltabs-popup binding
+       This binding relies on the structure of the tabbrowser binding.
+       Therefore it should only be used as a child of the tabs
+       element (in this case, when it is an anonymous node of <tabbrowser>).

Which case is "this case"? ;) "both cases" in the comment above the close-button binding refers to the two placements in which the button can be placed.

BTW and FWIW, unlike the close-button binding, the only requirement for this binding is being a child of the tabs element, see below.

+      <method name="_onShowingAllTabsPopup">
+        <body><![CDATA[
+          // clear out the menu popup
+          while (this.hasChildNodes())
+            this.removeChild(this.lastChild);


Might want to move this to the popuphiding handler, OK with me either way.

+          // set up the menu popup
+          var tabcontainer = document.getBindingParent(this);
+          var tabbrowser = document.getBindingParent(tabcontainer);
+          var tabs = tabbrowser.mTabs;
+

"mTabs" is the childNodes of the tabcontainer, so make this:

+          var tabcontainer = document.getBindingParent(this);
+          var tabs = tabcontainer.childNodes;


+          for (var i = 0; i < tabs.length; i++) {
....
+            var URI = tabbrowser.getBrowserForTab(curTab).currentURI.spec;

make this
  var URI = curTab.linkedBrowser.currentURI.spec;


Index: themes/winstripe/global/tabbox.css
===================================================================

Don't checkin this change.

I noticed you've not listed yourself in the contributors list of tabbrowser.xml.


r=mano otherwise.
Attachment #229782 - Flags: review?(bugs.mano) → review+
Attachment #229782 - Attachment is obsolete: true
about my last patch:

1) 

> nit: make this:
>+          <xul:menupopup class="tabs-alltabs-popup"
>+                         anonid="alltabs-popup"
>+                         position="after_end"/>

fixed, thanks.

2)

> nit: make this:
>+          pb2.addObserver("browser.tabs.disableBackgroundClose", 
>+                          this._prefObserver, true);
>+          pb2.addObserver("browser.tabs.closeButtons", 
>+                           this._prefObserver, true);

fixed, thanks.

3)

>..
>+  <!-- alltabs-popup binding
>+       This binding relies on the structure of the tabbrowser binding.
>+       Therefore it should only be used as a child of the tabs
>+       element (in this case, when it is an anonymous node of <tabbrowser>).
>
>Which case is "this case"?  ;)  "both cases" in the comment above the
>close-button binding refers to the two placements in which the button can be
>placed.
>
>BTW and FWIW, unlike the close-button binding, the only requirement for this
>binding is being a child of the tabs element, see below.

fixed, thanks.  my comment is now:

  <!-- alltabs-popup binding
       This binding relies on the structure of the tabbrowser binding.
       Therefore it should only be used as a child of the tabs element.
       This binding is exposed as a pseudo-public-API so themes can customize
       the tabbar appearance without having to be scriptable
       (see globalBindings.xml in Pinstripe for example).
  -->

4)

>+      <method name="_onShowingAllTabsPopup">
>+        <body><![CDATA[
>+          // clear out the menu popup
>+          while (this.hasChildNodes())
>+            this.removeChild(this.lastChild);
>
>Might want to move this to the popuphiding handler, OK with me either way.

fixed, thanks.  In the popuphiding handler, I'm now doing:

      <method name="_onHidingAllTabsPopup">
        <body><![CDATA[
          // clear out the menu popup and remove the listeners
          while (this.hasChildNodes()) {
            var menuItem = this.lastChild;
            menuItem.removeEventListener("command",
                                         this._allTabsMenuItemCommandHandler,
                                         false);
            this.removeChild(this.lastChild);
          }
        ]]></body>
      </method>

5)

>"mTabs" is the childNodes of the tabcontainer, so make this:
>
>
>+          var tabcontainer = document.getBindingParent(this);
>+          var tabs = tabcontainer.childNodes;

fixed, thanks.

6)

>make this
>  var URI = curTab.linkedBrowser.currentURI.spec;

fixed, thanks.

7)

>Index: themes/winstripe/global/tabbox.css
>===================================================================
>
>Don't checkin this change.

fixed, I've removed that from my tree.

8)

> I noticed you've not listed yourself in the contributors list of
> tabbrowser.xml

not fixed, maybe after some major contributions.
two more change coming:

1)

-this.removeChild(this.lastChild);
+this.removeChild(menuItem);

2)

a tooltip from beltzner
(In reply to comment #175)
> 2)
> 
> a tooltip from beltzner

I'd suggest "List all tabs" or "Show all tabs". The latter is what Safari does, but it seems odd to me since you're not actually showing the tabs, you're just showing the names of the tabs (which I think of as "listing").

Also, when you post the patch, could you (pretty please) run down pkasting's list from attachment 229310 [details] and indicate which bits the patch includes and doesn't? That will help us all test and work towards refinement.


Attached patch updated patchSplinter Review
two more changes, per asaf:

1)

< +   -   Seth Spitzer <sspitzer@mozilla.org>

2)

< +            this.removeChild(menuItem);
---
> +            this.removeChild(this.lastChild);

I'm going to land this on the trunk (I have r=mano), and then log all the spin off bugs.  (there are several)

I'll also go over pkastings design doc (v4) and list which issues I've fixed, and which ones I haven't.
Attachment #229842 - Attachment is obsolete: true
Summary: usability problems with tab overflow / scrolling → add "all tabs" menu to tabstrip to address usability problems with tab overflow / scrolling
fixed on the trunk, but not branch yet.

now to log spin off bugs, and go over pkastings list.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Looks really fine!
here's pkastings list, annotated:

>P1 (MUST HAVE by B2, these form/block the basic implementation):
>  1A. Tab strip does not have scrollbuttons [Exclusive with item CD]

not fixed.  The scrollbuttons are still part of the UI.  I saw a bugzilla
comment from mconnor ("assuming scroll indicators stay") so perhaps this is still being discussed.

>  1B. Tab strip has a menu button on its right end [see also item 3A]

fixed.

>  1C. Menu button is always present and enabled when the tab strip is present

fixed.

>  1D. Menu button looks like a button and shows targetable area at all times

fixed, but pkasting, please log a bug if the toolbarbutton is not what you had in mind.

>  1E. Menu button is large enough to easily target [see item UA]

fixed, but pkasting, please log a bug if the toolbar button isn't large enough.

>  1F. Menu button interacts cleanly, visually, with surrounding visual elements
>      (edge of tab, thin line/strip at bottom of tab strip)

fixed, but pkasting, please log a bug if the interaction isn't clean enough.

>  1G. Menu button causes a dropdown to appear which functions just like
>      dropdowns on back/forward buttons, i.e. click-to-dismiss

fixed.

>  1H. Dropdown lists all tabs, using typical ^ and v arrows if list is too long
>      for screen [see item RA]

fixed.

>  1I. Dropdown maxwidth is large enough to show a significant portion of the
>      tab title [see item UA]

fixed.

>  1J. Dropdown entry for the current tab is distinguished by bold text

fixed.

>  1K. When an entry is chosen or the keyboard is used (tab navigation
>      shortcuts, or arrow keys on tab strip) to select a new tab, show its
>      contents in the content window immediately

fixed, except that there are no shortcuts.

>  1L. When an offscreen tab is chosen in any of the above ways, move the strip
>      immediately to put the tab onscreen [see also items 2K, 3G]

fixed.

>  1M. In its static state, the tab strip only shows complete tabs.  A partial
>      tab never appears at either end of the strip (unless the strip is
>      scrolling [see item 3G]).  If the strip width is not divisible by the tab
>      width, padding is inserted between the rightmost visible tab and the menu
>      button [see also item CC], so there is always a tab beginning at the very
>      left edge of the strip.

not fixed.

>  1N. When there are more tabs than fit on the strip, the strip shows as many
>      tabs as possible, and does not leave padding large enough to hold a tab
>      at the right end.  In other words, once the strip scrolls the last tab
>      into view, it does not scroll any further.

not fixed.

>  1O. UI elements appear in reversed order in RTL UI cases, as appropriate

fixed.

>  1P. Dropdown entries have favicons (or the default "no favicon" icon)

fixed.

>  2A. The dropdown list is right-aligned with the right edge of the menu button
>      (the mirror of what happens for back/forward history dropdowns)

fixed.

>  2B. Dropdown entry for the current tab is distinguished by a large bullet to
>      the left of the column of favicons [may be exclusive with item CA].  This
>      should be consistent with platform conventions [see item UD].

not fixed.

>  2C. When there is overflow, dropdown entries for visible/non-visible tabs are
>      distinguished by background coloring (see IE7 overflow solution)
>      [exclusive with items CA, CB].  Entries for non-visible tabs use the
>      normal background color, while entries for visible tabs are colored using
>      -moz-Dialog.  When there is no overflow, all entries use the normal
>      background color.

not fixed.

>  2D. If the dropdown list is too long for the screen, the list appears
>      pre-scrolled such that the entry for the current tab is as close as
>      possible to the center of the visible list (subject to making the list as
>      long as possible)

not fixed.

>  2E. Dropdown entries show keyboard shortcuts for the first, last, previous,
>      and next tabs.  In cases where these overlap, the previous/next shortcuts
>      are shown.  [See also item CF]

not fixed.

>  2F. When Drag-And-Dropping a tab, if the mouse pointer moves over the window
>      frame or beyond the side of the window, the tab strip scrolls that
>      direction, one tab at a time, until the mouse moves back into window
>      [see also item 3I]

not fixed.

>  2G. If a tab is dropped off-window, the normal dropped-off-window behavior is
>      preformed, the tab remains in its current order, and the strip snaps back
>      to its pre-drag position

fixed.

>  2H. When a new tab is opened as an offscreen, background tab, menu button
>      flashes (see visual refresh style for feed icon; may need more visibility
>      than that)

fixed.

>  2I. When there is overflow, menu button glows (see visual refresh style for
>      search engine chooser) [may be exclusive with item CD]

not fixed.

>  2J. When the menu button is hovered in the overflow or non-overflow states,
>      its appearance changes appropriately.

not fixed.

>  2K. When an offscreen tab is brought onscreen, it is positioned as close
>      as possible to Distance X [see items UB and CG] from the appropriate end
>      of the strip [but do not break item 1N]

not fixed.

>  2L. minWidth and clipWidth are set to more reasonable values now that this
>      overflow solution works (bug 344048)

not fixed, but coming soon.

>P3 (Nice to have in final if we have time)
>  3A. Menu button is to the left of the tab strip's "X" button in a Fx1.5-style
>      layout

fixed.

>  3B. When right-clicked, menu button presents a context menu containing all
>      items that are relevant to the whole tab stip

not fixed.

>  3C. The status bar shows the URL of the dropdown's current selection (on
>      hover/keyboard navigate)

not fixed.

>  3D. Drag-And-Dropping a tab to the menu button pops open the menu.  User can
>      then continue the drag into the menu.  Menu items are reordered such that
>      the dragged tab's entry is always under the cursor.  If the mouse drags
>      out of the popup, it closes and the normal Drag-And-Drop behaviors apply.

not fixed (and might not be possible on the mac until an underlying bug is fixed.)

>  3E. Dropdown supports internal Drag-And-Drop to reorder tabs (see Places code
>      which apparently implements this)

not fixed.

>  3F. Dropdown supports Drag-And-Drop out of dropdown to tab strip

not fixed.

>  3G. When bringing an offscreen tab onscreen, instead of immediately moving to
>      the desired tab, the strip scrolls.
>      * Scrolling is smooth, not tab-at-a-time (but never stops with a partial
>        tab onscreen [see item 1M])
>      * While the desired tab is completely offscreen, the strip scrolls at
>        high speed [see below]
>      * Once the desired tab begins to come onscreen, the scroll rate ramps
>        down so that the tab decelerates to a stop.  The deceleration takes a
>        very short period of time to happen (certainly less than half a
>        second).
>      * The initial scroll speed is calculated to ensure the entire scroll
>        takes less than 0.75s.  There is a high minimum value.  Additionally,
>        there is a maximum time that the entire pre-deceleration scroll can
>        take.  The distance to scroll is divided by this time, and if the
>        resulting rate is higher than the minimum, that rate is used instead.

not fixed.

>  3H. If the tab bar is still scrolling but the user selects a new tab by
>      clicking the bar or using a keyboard shortcut, the chosen tab is selected
>      immediately [see item 1K], and the strip scrolls it into place.  This may
>      involve changing the scroll speed and/or reversing direction.  If the
>      selected tab is already at least partly onscreen, it begins decelerating
>      to stop as soon as possible.  If it cannot be decelerated in time to
>      be fully onscreen, the scroll direction reverses as needed so that the
>      tab decelerates back onto the screen.

not fixed.

>  3I. When dragging a tab off the window [see item 2F], the strip scrolls
>      smoothly, and decelerates when the mouse moves back into the window
>      * The initial scroll speed is much lower than in item 3G
>      * When the user has held the dragged tab off the edge of the strip for
>        2s, the scroll accelerates to the normal high-speed scroll

not fixed.

>  3J. Right-clicking a dropdown entry opens the context menu for that tab

not fixed.

>  3K. Hovering a dropdown entry shows a preview of that tab in a lightbox

not fixed, but I think vlad is interested in this.
Maybe I've missed something ... Why is the 'pressed' icon of alltabs.png (winstripe) used for the :hover state? Doesn't look right to me.
Whiteboard: [mustfix] → [mustfix][fixed on trunk, will bake before seeking branch approval]
*** Bug 344171 has been marked as a duplicate of this bug. ***
Comment on attachment 229870 [details] [diff] [review]
updated patch

this has r=mano

I pinged ispiked, and we are both ok with the one day of trunk baking is has gotten.

I'd like to get it on the branch for more testing.

there has been a couple more spin off bugs (fix winstripe css, tabstrip height is bigger now), but nothing that should prevent this from going on the branch, IMO.

seeking driver approval
Attachment #229870 - Flags: review+
Attachment #229870 - Flags: approval1.8.1?
Depends on: 345429
I'd very much like the preview feature (with some graying/whiting out the current tabs content in background).

I'd be also nice if the tab connected to the mouseovered list entry in the dropdown is highlighted. 

This could be: 
 - coloring the border
 - setting a background color(In reply to comment #181)

in reply to Comment #181: 
> Maybe I've missed something ... Why is the 'pressed' icon of alltabs.png
> (winstripe) used for the :hover state? Doesn't look right to me.
Reason for my bug 345429?

@all:
Thanks for that cool feature, it's gonna be so awesome with all the stuff from the list.
Blocks: 328065
No longer depends on: 328065
No longer depends on: 345429
Comment on attachment 229870 [details] [diff] [review]
updated patch

a=mconnor on behalf of drivers for checkin to the 1.8 branch
Attachment #229870 - Flags: approval1.8.1? → approval1.8.1+
fixed on the branch
Keywords: fixed1.8.1
Whiteboard: [mustfix][fixed on trunk, will bake before seeking branch approval] → [mustfix]
I can't tell if I should expect for this to be fixed yet or not but I still get partial tabs, which IMO looks horrible.

~B
Blocks: 343719
'All tabs' menu should probably also contain 'recently closed tabs' menu, makes it all the more intuitive and discoverable
(In reply to comment #187)
> I can't tell if I should expect for this to be fixed yet or not but I still get
> partial tabs, which IMO looks horrible.

From comment 180, design doc item 1M (don't show partial tabs) is not fixed.  I agree with your assessment, FWIW.

(In reply to comment #188)
> 'All tabs' menu should probably also contain 'recently closed tabs' menu, makes
> it all the more intuitive and discoverable

Why would I look in a list of open tabs to find a closed tab?  Also this breaks the same sorts of things about that dropdown that adding "Show All Tabs" breaks.  I would consider this extension fodder for similar reasons why I was convinced that was extension fodder.
Opened bug 345741 for a possible solution to design doc item 2B (distinguish active tab) and 2C (distinguish visible tabs).  Bug report includes mock-up image.
Depends on: 346006
Depends on: 346051
Blocks: 347363
Just to note here, items 3G, H and I (the smooth tab scroll aesthetics) are being taken care of in bug 347363, though most likely it will remain a trunk bug.
No longer blocks: 344579
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: