626903 - The "List all Tabs" menu should visually identify which tabs are on-screen (rather than scrolled off)

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

The List all Tabs menu should visually display the set of tabs on screen.  This will help users to quickly identify the areas outside of the current set of tabs displayed (that likely contain the tab that they are actually looking for).  I'll follow up later with mockups of the visual design.

Comment 1

[Frank Yan (:fryn)]

Visual design, please!

We'll need to do something different on OS X, e.g. show a dot for tabs on screens (we show a checkmark for the selected tab), since we can't change the background color of the menu items without making them non-native.

Comment 2

[Frank Yan (:fryn)]

Attachment: CSS-only patch!

It turns out that we can change the color of the native menu items on OS X!

Comment 3

[Frank Yan (:fryn)]

Attachment: screenshot w/ patch on Windows

Comment 4

[ithinc]

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-syncui.js#142

161     // Fake the tab object on the menu entries, so that we don't have to worry
162     // about removing them ourselves. They will just get cleaned up by popup
163     // binding.
164     menuitem.tab = { "linkedBrowser": { "currentURI": { "spec": label } } };

The "Tabs From Other Computers" menuitem is faking the tab property, which may show an undesirable shadowed item?

Comment 5

[Frank Yan (:fryn)]

(In reply to comment #4)
> The "Tabs From Other Computers" menuitem is faking the tab property, which may
> show an undesirable shadowed item?

I just addressed this in bug 653655 comment 2. Thanks.

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 529042 [details]
screenshot w/ patch on Windows

looks good, few quick thoughts:

-should we do nothing when there isn't any overflow?  just have a darker area seems like it would look strange.

-should we round the corners to convey more of a contiguous section (user may not know what it is initially and wonder if a tab alone by itself could also be darker).

Comment 7

[Frank Yan (:fryn)]

(In reply to comment #6)
> -should we do nothing when there isn't any overflow?  just have a darker area
> seems like it would look strange.

The patch already intentionally does exactly this. :)


> -should we round the corners to convey more of a contiguous section (user may
> not know what it is initially and wonder if a tab alone by itself could also be
> darker).

Yeah, I wanted to do that, but it's quite difficult to do code-wise (messing around with native menus is already hacky). I'll see if I can figure something out.

Comment 8

[Frank Yan (:fryn)]

(In reply to comment #7)
> (In reply to comment #6)
> > -should we round the corners to convey more of a contiguous section
> 
> Yeah, I wanted to do that, but it's quite difficult to do code-wise (messing
> around with native menus is already hacky). I'll see if I can figure something
> out.

I hacked at it, and it doesn't seem be do-able without making the menus non-native.

An alternative solution is to create a shadow-like gradient border on the top and bottom of the contiguous section. This can be done but would require a significantly more complex patch, e.g. touching a bunch of JS. (This patch is currently CSS-only!)

Comment 9

[Justin Dolske [:Dolske]]

Comment on attachment 529041 [details] [diff] [review]
CSS-only patch!

Clever, but I think ultimately a bit too much of a hack. I'd be willing to live with it, though, if someone who works in Theme more (like Dao) was ok. Would need an explicit comment, though. :)

Couple of ideas for other ways of doing this:

* Change the <menuitem> binding to have another box wrapping the content, and apply the background color there. That mostly works, but filling the gaps between <menuitem>s might be a bit tricky (would need some JS at least).

* Change the native theme code to support a derivative of -moz-appearance: menuitem that's hilighted, and paint appropriately.

* Maybe use a single semitransparent XUL box, absolutely positioned and with the right height/width to cover the visible-tab <menuitems>, and z-order it between the text and background? I suspect this would be a major headache to do, if it's even possible in XUL. :/

Comment 10

[Frank Yan (:fryn)]

(In reply to comment #9)
> Comment on attachment 529041 [details] [diff] [review] [review]
> CSS-only patch!
> 
> Clever, but I think ultimately a bit too much of a hack.

:(

> * Change the <menuitem> binding to have another box wrapping the content,
> and apply the background color there. That mostly works, but filling the
> gaps between <menuitem>s might be a bit tricky (would need some JS at least).
> 
> * Change the native theme code to support a derivative of -moz-appearance:
> menuitem that's hilighted, and paint appropriately.

I think this is too much work for a local enhancement and does not make the code more maintainable. Unless it is foreseeably certain that there are many other cases for which we'd want menuitem highlighting, I don't think this makes sense.

> * Maybe use a single semitransparent XUL box, absolutely positioned and with
> the right height/width to cover the visible-tab <menuitems>, and z-order it
> between the text and background? I suspect this would be a major headache to
> do, if it's even possible in XUL. :/

I don't think this is less hacky than my solution.
The idea is to style a _menuitem_ itself such that it is visually distinguishable from the other menuitems, so applying a style to the menuitem itself is sensible.
Also, my informed guess (after giving a try) is that if this were possible, then simply setting the background-color on the menu item would have worked.

I'm not sure what the concern is here. If the concern is maintainability, it's probably better not to add a lot more code at a lower level, e.g. the binding or the native theme code. Rather, I think commenting the code I added in the patch with something like "highlight the onscreen tabs" is sufficient.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Dolske: this doesn't look very hacky to me. What was the concern re: hackyness exactly?

Comment 12

[Justin Dolske [:Dolske]]

I think the main thing is just that it's a hack -- in any other context, you'd certainly use background-color. And so someone looking at this is going to be confused as to what's going on. There's also some potential perf issues -- pwalton found issues with gradients before, an I don't know how good our code is wrt large gradients clipped to a small area. (I'm actually not too concerned about the perf thing here, since it's in non-primary UI, and the number of menuitems getting this treatment is constrained to the number that fit on-screen.)

Like I said, though, I can live with it if you or Dao are OK with it. As far as hacks go, it's well-contained, not an obvious alternative, and can be commented to explain what's happening.

Comment 13

[Dão Gottwald [:dao]]

This is ok, box-shadow as implemented allows this intentionally. We're using it already: http://hg.mozilla.org/mozilla-central/annotate/b8a035ebdf0f/toolkit/themes/winstripe/global/findBar.css#l99

The 1000px seem excessive, though.

Comment 14

[Frank Yan (:fryn)]

(In reply to comment #13)
> The 1000px seem excessive, though.

Yeah. It simply needs to be at least half the shorter of the width and height. Percentages don't work here unfortunately. I just realized that using em makes a lot more sense. Would 1em or 2em be sufficient?

Comment 15

[Frank Yan (:fryn)]

Attachment: patch v2

Added comment explaining usage of box-shadow instead of background-color.
Replaced 1000px with 2em.
(1em should be sufficient for default font size, but using 2em to be safe for bizarre font settings. If you think 1em is enough, that works for me too.)

Comment 16

[Frank Yan (:fryn)]

Attachment: hg export of patch v2 for checkin

Comment 17

[Frank Yan (:fryn)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/85b61a2b2da5

Comment 18

[George Carstoiu]

 Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110628 Firefox/7.0a1

Verified on Ubuntu 11.04 x86, Mac OS X 10.6, Win7, WinXP using following steps:
 1. Open at least 20 tabs 
 2. Go to List all tabs button and observe how list is displayed

Setting status to Verified Fixed.