bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Improve "All Tabs" menu active/visible tab feedback

RESOLVED DUPLICATE of bug 626903

Status

()

Firefox
Tabbed Browser
--
enhancement
RESOLVED DUPLICATE of bug 626903
12 years ago
7 years ago

People

(Reporter: Michael Clackler, Assigned: Michael Ventnor)

Tracking

2.0 Branch
x86
Windows XP
Points:
---
Bug Flags:
blocking-firefox2 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [expired?])

Attachments

(7 attachments, 9 obsolete attachments)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060724 BonEcho/2.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060724 BonEcho/2.0b1

The "all tabs" drop-menu should highlight the current visible tabs.  This has been discussed at length in Bug 343251.

Reproducible: Always

Steps to Reproduce:
1. Open enough tabs to implement "overflow"
2. click the "all tabs" button


Actual Results:  
A list of tabs is provided with the active tab in bold.

Expected Results:  
A list of tabs is provided providing feedback of both the active tab and visible tabs.

There have been several ideas introduced including highlighting the background of the visible tabs and adding a check or bullet next to the active tab.  Each of these has had their negative points brought to light including:
1.  The visible tabs background highlight might interfere with the selection highlight.
2.  What should the threshold be for turning on the background highlighting?
3.  The addition of a check or bullet to the front of the menu item takes up vertical space for all menu-items.

I noticed Google groups has implemented a system in their post sidebar which indicates both the selected post and the range of posts visible on the current screen.  This is done without background highlighting, without a check or bullet, and only taking 6 pts of vertical space.  There are thin vertical bars which indicate the visible messages, and an arrow-head indicating the selected message.

I think a system similar to this would effectively implement/improve user-feedback for the both the visible and active tabs without the negative points of the other options brought to light in bug 343251.

Mock-up image to come...
(Reporter)

Comment 1

12 years ago
Created attachment 230452 [details]
mock-up of possible menu feedback

This is a mock-up of the Google groups method of feedback on the "all tabs" menu.  

1.  The visible tabs are highlighted with the vertical orange bar.
2.  The active tab is highlighted with the arrow.
3.  The background color of the menu items does not change and the system has no effect of the selection highlight (selection highlight seen in mock-up).
4.  The overall width of the menu has not changed.  The system required 6 pts to the left of the favicon which I took from the right hand side of the menu (there seems to be too much white space there anyways...)
5.  The added 6 pts of vertical space by the favicon is very small and used for more than just highlighting the active tab (ala check or bullet) with the additional icons for the visible tabs.  This greatly reduces the argument of this being wasted space.
6.  The system could easily be "always on" removing the question of what threshold to turn on the background highlighting.
(Reporter)

Comment 2

12 years ago
Seth,

Please look at the mock-up image.  I think you will see this system of feedback might overcome most of the negatives brought to light for the other options. 
Changing "Severity" to "enhancement" as this is a change to an existing feature... probably the best category for it.
Severity: normal → enhancement

Comment 4

12 years ago
My reaction to the screenshots is that it's too subtle to be useful, but I'd be happy to see a working demo.

It might be better to use a solid vertical line that extends down beside all visible tabs, instead of the vertical-line-on-each-item look.  (In other words, fill the gaps between lines.)  I'm willing to believe that is insanely hard to do technically, though.
(Reporter)

Comment 5

12 years ago
(In reply to comment #4)
> My reaction to the screenshots is that it's too subtle to be useful, but I'd be
> happy to see a working demo.

I might say that the simple, subtle, unobtrusive nature of the system is one of its strong points.  With the arrow head pointing to the bold active tab title, it is easily understood this is the active tab.  I feel the arrow is much better at this than a check or bullet.  The system also differentiates the visible tabs with icons instead of using background shades which could cause problems with those living with certain forms of color-blindness.

I don't have the knowledge to get a working demo up quickly.  It appears from the various screenshots in bug 343251 that Seth might be the best person to quickly get a working example.  I can easily supply the png images for the arrow head and bar if needed.  To get a quick feel of the system, go to http://groups.google.com/group/mozilla.dev.themes/browse_frm/thread/5b031aec9446fc0c?scoring=d&  and look in the sidebar for the system implementation.

> It might be better to use a solid vertical line that extends down beside all
> visible tabs, instead of the vertical-line-on-each-item look.  (In other words,
> fill the gaps between lines.)  I'm willing to believe that is insanely hard to
> do technically, though.

I'll add a second mock-up image with slightly thicker and taller lines.
(Reporter)

Comment 6

12 years ago
Created attachment 230494 [details]
mock-up ver 2

In order to help the "subtleness", I made the icons slightly thicker and changed the height of the vertical bars to equal the height of the favicon.  This is probably the tallest the bars can be while keeping the code changes relatively small.
(Reporter)

Comment 7

12 years ago
Created attachment 230504 [details]
mock-up ver 3 - vertical tab-like

Tinkering around with the "solid vertical line" idea, I started to lose the idea that the bars were indicating tabs.  Thinking further about the idea that the bars were icons, I slightly rounded the outer corners of the bars to make them appear slightly like a vertical tab.  This change improved the visual link between the system and the tab bar.  I think it is much more apparent with this change that the bars indicate tabs.  There is also a visual cue with the number of bars equaling the number of visual tabs.  If this was a solid line, the link with the tab bar would be much weaker.
(Assignee)

Comment 8

12 years ago
Created attachment 230554 [details]
Screenshot of my patch

Hmm... I think we can only put in one image per menuitem, correct me if I'm wrong. I worked on a little something that gives the IE7'esque look to the menu, though.
(Assignee)

Comment 9

12 years ago
Created attachment 230557 [details] [diff] [review]
Patch v1

OK, here is the patch. It gives an IE7 look to the menu. It only happens when there is overflow, and when a significant portion of the tab is visible. Its item gets coloured blue when the menu is built each time.
Attachment #230557 - Flags: ui-review?(beltzner)
Comment on attachment 230557 [details] [diff] [review]
Patch v1

hardcoding colors is bad bad bad!  what if the text color in the user theme was blue?

need to find an nsITheme color that'd work in this case.  I'm not sure this adds anything to the current impl, as I noted in bug 343251
Attachment #230557 - Flags: review-
confirming.

> I think we can only put in one image per menuitem

see https://bugzilla.mozilla.org/attachment.cgi?id=229260

the way I did that was to create a new menuitem binding that has two images.  (in that case, the first image is the radio image, the second is the favicon.)

but, heads up, this could get "wontfix".  

ben, beltzner, mconnor: can you confirm we want to do this?  (mconnor, at least, rejected this in bug #343251.)
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 12

12 years ago
(In reply to comment #7)
> Created an attachment (id=230504) [edit]
> mock-up ver 3 - vertical tab-like
> 
> Tinkering around with the "solid vertical line" idea, I started to lose the
> idea that the bars were indicating tabs.  Thinking further about the idea that
> the bars were icons, I slightly rounded the outer corners of the bars to make
> them appear slightly like a vertical tab.

I really like this change, and I agree with you that icons are better than colors in a lot of theoretical ways (though I'm still not 100% sold on the way it "feels" to look at).  I believe some of mconnor's original objections to this sort of distinction were based on concerns that users couldn't use the visible tab strip alone to distinguish visible tabs.  I don't know if the recent patch to change tab minwidth to 100px modifies that.  In your screenshot above, the large number of small visible tabs marginalizes the usefulness of this, but maybe in a 100px world it's better.

(In reply to comment #10)
> hardcoding colors is bad bad bad!

Agreed.

(In reply to comment #11)
> ben, beltzner, mconnor: can you confirm we want to do this?  (mconnor, at
> least, rejected this in bug #343251.)

See comments above about maybe minwidth changes changing the usefulness here.

(BTW, I don't know if this bug is worth doing now, since I'm still just as concerned about scrollbuttons and drawing partial tabs as I am about this, and I don't know how much time we have left to get things done.  The current state of affairs still feels to me like "better, but not yet right" so I'd like to see progress in at least some area.  However, I'd really like to see this bug get tackled on trunk at least even if it doesn't make it to branch.)
(Reporter)

Comment 13

12 years ago
(In reply to comment #12)
> In your screenshot
> above, the large number of small visible tabs marginalizes the usefulness of
> this, but maybe in a 100px world it's better.
> 
> See comments above about maybe minwidth changes changing the usefulness here.

I'll add another mock-up with the 100px setting for comparison.

> (BTW, I don't know if this bug is worth doing now, since I'm still just as
> concerned about scrollbuttons and drawing partial tabs 

See bug 345884 for an idea that would eliminate drawing partial tabs on the left side of the tab bar.

(Reporter)

Comment 14

12 years ago
Created attachment 230611 [details]
mock up ver 4 - 100px tab width

Same image as mock-up ver. 3 except the min tab width setting is 100px.  I also did not add the tab icon for the partial tab on the right side. This reduced the tabs requiring icons from 12 to 7.
(Reporter)

Comment 15

12 years ago
Created attachment 230617 [details]
Mock up Active Tab icon

Adding icons in case anyone knows how to do what was mentioned in comment #11.
(Reporter)

Comment 16

12 years ago
Created attachment 230618 [details]
Mock up Visible Tab icon
The simplest solution here is to do something like this:

Set the background color of the visible tabs to AppWorkSpace (dark gray). Set the text to HighlightText (white)

This creates a box of shading around the visible region. Highlight background color remains Highlight or whatever it is. I don't think the fact that the highlight color doesn't change depending on whether or not the selected item is visible is a big deal at all. 

No need for crazy icons or anything scary. Set attributes and stlye appropriately. 

Comment 18

12 years ago
(In reply to comment #17)
> This creates a box of shading around the visible region. Highlight background
> color remains Highlight or whatever it is. I don't think the fact that the
> highlight color doesn't change depending on whether or not the selected item is
> visible is a big deal at all. 

I don't either, and after much peering around at the various mockups, I think that IF we distinguish visible tabs, then we should use color/contrast like this rather than icons.  The icons still seem too subtle, too cluttered, and too confusing when I look at them, even when I know exactly what I'm looking for.  The fact that it would probably be easier to code Ben's suggested change is an added bonus.
(In reply to comment #18)
> I don't either, and after much peering around at the various mockups, I think
> that IF we distinguish visible tabs, then we should use color/contrast like

Agreed. Colour > icons; the latter add clutter that distract from the primary task of finding the tab you're looking for.

On that point, and as hinted at by seth and pkasting: I'm still not sure what problem a user faces that's solved by providing this information. It provides some subtle hints and context, but the user can see the set of tabs that's currently visible in the tabstrip already by, uh, looking.

There's two cases where I can see this treatment as being useful. 1) if the user changes tabsMinWidth to be < 100px such that they aren't really seeing anything useful in the tabstrip, or 2) if the set of highlighted tabs followed mouse focus in the menu, thereby illustrating the set of tabs that *would* be visible if the user were to select the tab they're hovering over.

Michael: what's the problem you're trying to solve for users here?
(Assignee)

Comment 20

12 years ago
Created attachment 230711 [details] [diff] [review]
Patch, softcoded colors

OK, I found the colour InactiveCaption to work best with both Windows and Mac. (I originally wanted InactiveCaptionText because it looked much better on Windows, but it caused unreadability on Mac) It produces a blue similar to what I had before.

@Beltzner, I suppose one use of this is to see what tabs are outside of the strip without needing to click the scroll buttons. If you have lots of tabs it can get unclear which tabs are there and which tabs aren't. Plus it keeps us in line with IE7. :)
Attachment #230557 - Attachment is obsolete: true
Attachment #230711 - Flags: ui-review?(beltzner)
Attachment #230711 - Flags: review?(mconnor)
Attachment #230557 - Flags: ui-review?(beltzner)
(Assignee)

Comment 21

12 years ago
Created attachment 230722 [details] [diff] [review]
Patch, increase tab visibility required

This patch just increases the amount of tab visibility required for its menu item to be highlighted to 70%.
Attachment #230711 - Attachment is obsolete: true
Attachment #230722 - Flags: ui-review?(beltzner)
Attachment #230722 - Flags: review?(mconnor)
Attachment #230711 - Flags: ui-review?(beltzner)
Attachment #230711 - Flags: review?(mconnor)

Comment 22

12 years ago
(In reply to comment #19)
> Michael: what's the problem you're trying to solve for users here?

Since I had specced this in my design, I can at least give my rationale for the feature, from bug 343251 comment 79:

"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."

It's really the second point I was aiming at, but if the first point matters at all, then it matters more now than it did at the time since we no longer have a bullet on the item, just bold.

(In reply to comment #20)
> OK, I found the colour InactiveCaption to work best with both Windows and Mac.

I'm very concerned with picking theme colors just because they look good.  That tends to break badly with non-default themes, because you're adding a color usage for those items that the theme author didn't originally test.

To put it another way, you should use "InactiveCaption" if your item has something to do with inactive captions.

Ben's suggestions like "AppWorkSpace" use the colors for purposes closer to their original intent/meaning, and thus are more likely to work with other themes.  (And, perhaps, more likely to be visually uniform and unconfusing to users than when random colors mean different things in different places.)

(In reply to comment #21)
> This patch just increases the amount of tab visibility required for its menu
> item to be highlighted to 70%.

Why not 100%?  Anything less seems like the menu behavior is arbitrary.
(Assignee)

Comment 23

12 years ago
> I'm very concerned with picking theme colors just because they look good.  That
> tends to break badly with non-default themes, because you're adding a color
> usage for those items that the theme author didn't originally test.
> To put it another way, you should use "InactiveCaption" if your item has
> something to do with inactive captions.
> Ben's suggestions like "AppWorkSpace" use the colors for purposes closer to
> their original intent/meaning, and thus are more likely to work with other
> themes.  (And, perhaps, more likely to be visually uniform and unconfusing to
> users than when random colors mean different things in different places.)

I guess, its just that the default AppWorkSpace colour looked... not pleasing. But I'll do it.

> Why not 100%?  Anything less seems like the menu behavior is arbitrary.

Use the tab scroll buttons to hide the leftmost tab, then use the All Tabs menu to select that first tab. At least on my version, a very tiny bit of the tab is still not visible. By using 100%, this tab won't get highlighted. Thats why I lowered it from 100% before making this patch. I could increase it further if you wish, but using 100% could cause a bit of unwanted exclusion.

Comment 24

12 years ago
(In reply to comment #23)
> Use the tab scroll buttons to hide the leftmost tab, then use the All Tabs menu
> to select that first tab. At least on my version, a very tiny bit of the tab is
> still not visible.

Can you file a bug on that against sspitzer and post a screenshot?  When you select a tab with the All Tabs menu it's supposed to ensure the tab is 100% visible.

With that bug fixed, I think my comment still stands.
(Assignee)

Updated

12 years ago
Depends on: 346051
(Assignee)

Updated

12 years ago
Attachment #230722 - Flags: ui-review?(beltzner)
Attachment #230722 - Flags: review?(mconnor)
(Assignee)

Comment 25

12 years ago
(In reply to comment #24)
> (In reply to comment #23)
> > Use the tab scroll buttons to hide the leftmost tab, then use the All Tabs menu
> > to select that first tab. At least on my version, a very tiny bit of the tab is
> > still not visible.
> 
> Can you file a bug on that against sspitzer and post a screenshot?  When you
> select a tab with the All Tabs menu it's supposed to ensure the tab is 100%
> visible.
> 
> With that bug fixed, I think my comment still stands.
> 

Filed bug 346051.
(Assignee)

Comment 26

12 years ago
Created attachment 231191 [details] [diff] [review]
Cleaner patch

I had a quick perusal through the ensureElementIsVisible code and I saw nothing which specifically caused bug 346051. 

In this patch, the offset will now only apply if all of the conditions of bug 346051 are met. Otherwise 100% of the tab must be visible.

I really want to get this on the branch before its too late.
Attachment #230722 - Attachment is obsolete: true
Attachment #231191 - Flags: review?(mconnor)
(Reporter)

Comment 27

12 years ago
Requesting blocking 2.0 to get on radar.  Patch exists and much-requested feature would provide "expected" parity with many other browsers.
Flags: blocking-firefox2?
So, we shouldn't care about some theoretical parity with IE7.  If something isn't useful, we won't do it just because someone thinks we should, or because someone else does it.

I don't think anyone's given a compelling enough reason to reconsider the original thinking, there's the obvious reasoning (helping find stuff not on the tabstrip), but I think the behaviour has a negative effect of segmenting the list instead of it being simple and clean and skimmable.  FWIW, I find my eyes go to the highlighted set first when playing with IE7, which makes the "find the offscreen tab" case worse, since you're drawn to the current tabset first.

I'm going to clear the review request until someone makes a better case, but not going to WONTFIX this yet.
Flags: blocking-firefox2? → blocking-firefox2-

Updated

12 years ago
Attachment #231191 - Flags: review?(mconnor)
(Reporter)

Comment 29

12 years ago
(In reply to comment #28)
> So, we shouldn't care about some theoretical parity with IE7.  If something
> isn't useful, we won't do it just because someone thinks we should, or because
> someone else does it.

I personally find the feature useful.  I also worry about the feeling future IE users will have when they try Firefox, find a very familiar UI in the "All tabs" menu, and come away with the feeling that "somethings missing".  I'm not saying IE7 is better by any means, but it will be "forced" onto users through windows update and the UI features will quickly become "what's expected" by many people.

> I don't think anyone's given a compelling enough reason to reconsider the
> original thinking, there's the obvious reasoning (helping find stuff not on the
> tabstrip), but I think the behaviour has a negative effect of segmenting the
> list instead of it being simple and clean and skimmable.  FWIW, I find my eyes
> go to the highlighted set first when playing with IE7, which makes the "find
> the offscreen tab" case worse, since you're drawn to the current tabset first.

Maybe it's the way I think, but I find this statement a little confusing.  If I have a list of information in which I am looking for a specific item, the first thing I do is try and figure out if there is any way I can reduce the list.  Your explanation of "segmenting the list" is exactly what is desired.  By segmenting the tab strip into three possible segments (left tabs, visible tabs, right tabs), the list of tabs you have to "skim" is greatly reduced in many cases.

Assuming you have many tabs open and are in the middle of the tab range:
1.  You generally have an idea before going to the tab menu if the tab you are looking for is before of after the tabs you are looking at.  As you have explained, "I find my eyes go to the highlighted set first", so you know what is currently visible in the list.  With this knowledge, and the "idea" that the tab is either before or after this range of tabs, you have just reduced the amount of tabs you need to "skim" by more than 50% and greatly decreased the required "skim" time.
2.  If you have many tabs open from the same site, the Favicon and beginning of the title is the same for all tabs.  With many tabs open, the tab width has been decreased and all the tab bars on the screen look the same.  You know the tab you are looking for is one of the ones you currently see on the tab bar.  Opening the tab menu, you instantly know what tabs are currently visible and have a longer title string to be able to make a quick choice.
3.  You want to use the scroll buttons to change the range of visible tabs to bring a new tab into view while keeping the current active tab visible.  With the visible tabs segmented, you can quickly tell if this is possible without "moving" the tab, and how many tabs in what direction you need to scroll.  If you need to "drag" the tab, you know where in the tab order you need to place it to ensure it is visible.

There have been many more examples given by many people in bugs related to the "All Tabs" menu, and as you've mentioned "there's the obvious reasoning (helping find stuff not on the tabstrip)".  I'm not sure what the "original thinking" was, but since this feature reduces the required manual "scanning" of the tab list and increases the efficiency of the UI, I really hope we don't implement this just because IE7 has.
(Assignee)

Comment 30

12 years ago
Created attachment 243779 [details] [diff] [review]
Patch

OK, now I'll get cracking at this.
Fragmenting the menu with colours (or something else) is a good thing. I remember a Flash Card Game (didn't play it at work!) that utilized something similar so you'd know which cards were on screen right now and where cards you'd want to find were. I found this immensely helpful.

This is really useful UI. It isn't something just for IE7-parity. This patch introduces code-only changes so ui-r isn't necessary yet. All menu items which correspond to a currently visible tab will get the attribute isVisible. 

To decorate these items, you'd need to put something like this in browser.css:
.alltabs-item[isVisible="true"] {
  background-color: lightblue;
}
Attachment #231191 - Attachment is obsolete: true
Attachment #243779 - Flags: review?(mano)

Comment 31

12 years ago
(In reply to comment #30)
> Created an attachment (id=243779) [edit]
> Patch
> 
> OK, now I'll get cracking at this.
> Fragmenting the menu with colours (or something else) is a good thing. I
> remember a Flash Card Game (didn't play it at work!) that utilized something
> similar so you'd know which cards were on screen right now and where cards
> you'd want to find were. I found this immensely helpful.
> 
> This is really useful UI. It isn't something just for IE7-parity. This patch
> introduces code-only changes so ui-r isn't necessary yet. All menu items which
> correspond to a currently visible tab will get the attribute isVisible. 
> 
> To decorate these items, you'd need to put something like this in browser.css:
> .alltabs-item[isVisible="true"] {
>   background-color: lightblue;
> }
> 
In an effort to assist, two extensions dealing with colored tabs:
Colorful Tabs (http://varun21.googlepages.com/main.html)
HashColouredTabs (http://hashcolouredtabs.mozdev.org/)
Comment on attachment 243779 [details] [diff] [review]
Patch

>Index: tabbrowser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v
>retrieving revision 1.212
>diff -u -8 -p -r1.212 tabbrowser.xml
>--- tabbrowser.xml	27 Oct 2006 10:48:28 -0000	1.212
>+++ tabbrowser.xml	27 Oct 2006 11:52:09 -0000
>@@ -2961,16 +2961,23 @@
>               case "busy":
>               case "image":
>               case "selected":
>                 if (aEvent.attrChange == aEvent.REMOVAL)
>                   menuItem.removeAttribute(attrName);
>                 else
>                   menuItem.setAttribute(attrName, aEvent.newValue);
>             }
>+
>+            // selected is the only attribute which can affect this.
>+            if (attrName == "selected") {
>+              var self = this;
>+              setTimeout(function() { self._updateTabsVisibilityStatus(); }, 0);
>+            }
>+

You can scroll the tabstrip while the menu is open ;) I would rather add a scroll event handler (then you wouldn't need need the timer, probably).

>+      <method name="_updateTabsVisibilityStatus">
>+        <body><![CDATA[
>+          var tabstrip = document.getBindingParent(this).mTabstrip;
>+          // We don't want menu item decoration unless there is overflow.
>+          if (tabstrip._scrollButtonUp.collapsed && tabstrip._scrollButtonDown.collapsed)
>+            return;

or better:
   var tabcontainer = document.getBindingParent(this);
   if (tabcontainer.getAttribute("overflow") != "true")
     return;

>+
>+          tabstrip = tabstrip.scrollBoxObject;

nit: this isn't the tabstrip, this is the tabstrip's boxobject (read: use a different variable, something like tabstripBO).

>+          for (var i = 0; i < this.childNodes.length; i++) {
>+            var menuItem = this.childNodes[i];

nit: there's no need to "cache" the menuitem here.

>+            var curTab = menuItem.tab.boxObject;

ditto.

>+            if (curTab.screenX >= tabstrip.screenX &&
>+                curTab.screenX + curTab.width <= tabstrip.screenX + tabstrip.width) {

please test RTL UI.

>+              menuItem.setAttribute("isVisible", "true");

maybe name the attribute isTabVisible?
Attachment #243779 - Flags: review?(mano) → review-
(Assignee)

Comment 33

12 years ago
 
> You can scroll the tabstrip while the menu is open ;) I would rather add a
> scroll event handler (then you wouldn't need need the timer, probably).

But only through the mouse scroll wheel, which in turn changes the tab, unless there is another method I'm not aware of. The problem with setting up an event handler is that many scrolling functions call the tab scrolling function directly from nsIScrollBoxObject and since I don't know C++ I can't make an event handler without changing all the calls (and there are a LOT of them) to point to within the scrollbox.xml widget.

I'm not sure how to really address this, but I can definitely work on the other points.
(Assignee)

Comment 34

12 years ago
Created attachment 257208 [details] [diff] [review]
Patch 2

Please ignore everything I said previously. I knew a lot less about the toolkit codebase then than what I do now. :)

This is the back-end stuff. After this is checked in the actual UI can be implemented by the future Fx3 themers. Also the reason I removed the comment in this patch was because I fixed that issue a while ago (bug 345258).
Assignee: nobody → ventnor.bugzilla
Attachment #243779 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #257208 - Flags: review?(mano)
What if the window is resized in such a way that the tabbar underflows (e.g. when trying to maximize a small browser window)?
(Assignee)

Comment 36

12 years ago
(In reply to comment #35)
> What if the window is resized in such a way that the tabbar underflows (e.g.
> when trying to maximize a small browser window)?
> 

Can that even be done while the menu is open?
Not really, I forgot these handlers are only attached when the menu is open.
Comment on attachment 257208 [details] [diff] [review]
Patch 2

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

> 
>+      <method name="_updateTabsVisibilityStatus">
>+        <body><![CDATA[
>+          var tabContainer = document.getBindingParent(this);
>+          // We don't want menu item decoration unless there is overflow.
>+          if (tabContainer.getAttribute("overflow") != "true")
>+            return;
>+
>+          tabstripBO = tabContainer.mTabstrip.scrollBoxObject;

declare tabstripBO.

r=mano otherwise.
Attachment #257208 - Flags: review?(mano) → review+
(Assignee)

Comment 39

12 years ago
Created attachment 257340 [details] [diff] [review]
Nit fix
Attachment #257208 - Attachment is obsolete: true
(Assignee)

Updated

12 years ago
Whiteboard: [checkin needed]

Comment 40

12 years ago
mozilla/toolkit/content/widgets/tabbrowser.xml  1.224
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
(Assignee)

Comment 41

12 years ago
Not fixed yet. That patch was the backend, we still need to implement the actual UI in the themes. I'll temporarily use the suggestion from comment 17.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 42

12 years ago
OK, sorry.

FWIW, I think one patch per bug is a better approach for multiple reasons (easier to deal with regressions, smaller and more manageable bugs).
(Assignee)

Comment 43

12 years ago
Created attachment 257765 [details] [diff] [review]
Themes fix

Implements comment 17 for temporary use on Trunk. Sorry about the patch splitting.
Attachment #257340 - Attachment is obsolete: true
Attachment #257765 - Flags: review?(mano)
Comment on attachment 257765 [details] [diff] [review]
Themes fix

mconnor or beltzner should review this first.
Attachment #257765 - Flags: review?(mano)
(Assignee)

Comment 45

12 years ago
Comment on attachment 257765 [details] [diff] [review]
Themes fix

Are there any plans to do significant work on the theme for Firefox 3?

I was thinking of doing an All Tabs Button on this, adding an implementation to let everyone know that it exists, but letting the real themers spruce it up for release.
Attachment #257765 - Flags: ui-review?(beltzner)
(Assignee)

Comment 46

11 years ago
Created attachment 265502 [details] [diff] [review]
Better CSS fixes

This one uses not() so its smaller and less prone to redundancy.
Attachment #257765 - Attachment is obsolete: true
Attachment #265502 - Flags: ui-review?(beltzner)
Attachment #257765 - Flags: ui-review?(beltzner)

Comment 47

11 years ago
Not sure if this is the place, but the highlighting could be more distinguishable, and it would be nice to have both the x on the tab and at the end to close tabs. The one on the tab is nice for when there are a lot and you want to close individual ones, and the one at the end is nice if you want to do maybe 10 out of 20 in a row.
Seems like this bug has expired? The patch will have definitely bitrotted by now...
Status: REOPENED → NEW
Whiteboard: [expired?]
Version: unspecified → 2.0 Branch

Comment 49

8 years ago
browser.css has moved:
http://mxr.mozilla.org/mozilla-central/find?string=browser.css&tree=mozilla-central&hint=themes

At least on Linux, -moz-appearance seema to override background-color, so this needs further tweaking.

Comment 50

7 years ago
This should be fixed by Bug 626903, right?
(Or is there something missing; I didn't read all comments of this bug)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 626903

Updated

7 years ago
Attachment #265502 - Attachment is obsolete: true
Attachment #265502 - Flags: ui-review?(mbeltzner)
You need to log in before you can comment on or make changes to this bug.