1186632 - Use the overlay sound icon when the width of the tab drops below 140px

Status: ASSIGNED

(Firefox :: Tabbed Browser, defect)

Description

[(no longer active)]

See the discussion around bug 486262 comment 139.

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch

Comment 2

[(no longer active)]

I think the code changes are mostly fine, but this needs to get ui-review.  Let me upload screenshots...

Comment 3

[(no longer active)]

Attachment: Before tab overflow

Comment 4

[(no longer active)]

Attachment: After tab overflow

Comment 5

[(no longer active)]

We should at least decide what to do here before shipping.

Comment 6

[(no longer active)]

Attachment: Sound playing icon on overflowing tab without favicon

Hmm, this looks weird to me.  In this case, when the tab bar is not overflown, we display no tab icon, and then the tab suddenly gets a blank icon...

Maintaining the icon at the right side in this case doesn't seem to waste any more space, but this is of course more consistent with what happens when the tab does have a favicon, which is the majority of the cases.

Up to the UX team...

Comment 7

[(no longer active)]

Comment on attachment 8642028 [details] [diff] [review]
patch

Review of attachment 8642028 [details] [diff] [review]:
-----------------------------------------------------------------

I think given that this code is purely changing the presentation, the existing browser_audioTabIcon.js should be enough, but if you think it's worth it, feel free to extend that test to cover the tab bar overflow scenario as well.  We already execute the test "recursively" to test everything for the foreground and background tab case here: https://dxr.allizom.org/mozilla-central/source/browser/base/content/test/general/browser_audioTabIcon.js?offset=0#110.  A similar recursion can be used to test the tabbar overflown case.

::: browser/base/content/tabbrowser.css
@@ +19,5 @@
>  }
>  
>  .tab-close-button[pinned],
> +.tabbrowser-tabs[cliptabcontent] > * > * > * > .tab-close-button:not([visuallyselected="true"]),
> +.tab-icon-sound:not([soundplaying]),

Please incorporate bug 1190081 into this since I suspect that bug to land first.

@@ +31,5 @@
>  
> +.tab-icon-image[src],
> +.tab-icon-image[pinned],
> +.tab-icon-image[crashed],
> +.tabbrowser-tabs[cliptabcontent] > * > * > * > .tab-icon-image[soundplaying],

Depending on the UI review for attachment 8642077 [details], this may need to change...

Comment 8

[Stephen Horlander [:shorlander]]

Comment on attachment 8642076 [details]
After tab overflow

I don't think we should do this. 

We would be trading consistency and hit target size for a few extra characters in a few cases.

Comment 9

[Dão Gottwald [:dao]]

(In reply to Stephen Horlander [:shorlander] from comment #8)
> Comment on attachment 8642076 [details]
> After tab overflow
> 
> I don't think we should do this. 
> 
> We would be trading consistency

We're already moving the indicator this for pinned tabs. This is merely changing where we draw the line. And then we're also similarly trading consistency with the close buttons.

> and hit target size for a few extra characters

Just like we're doing for the close buttons, right? Although this case doesn't affect all tabs (only those playing sound), it doesn't really seem different in principle.

As noted before, we display about three (selected tab) or five (background tabs) characters when reaching the tab minimum width (100px). Removing another three characters from there is significant.

> in a few cases.

I'm not quite sure what you mean here. Tabs that won't have their titles cropped below the tab clip width seem rather rare, so it seems that we'd get extra characters for most cases, although the relative gain of those characters becomes greater the close we get to the minimum width. (Which again means that we need to draw a line somewhere, and 140 seems to have worked alright over the years.)

Comment 10

[Dão Gottwald [:dao]]

Attachment: screenshot showing how the current UI is broken

Comment 11

[(no longer active)]

Talking to shorlander, we don't think this is a blocker.

Comment 13

[magicp]

Attachment: Qute6++ tab-overflow-min-width-100px-screen-shot.png

As you may know, I am providing complete themes (Qute5++ and Qute6++). So I have modified UI design and released. Please refer to attached image.

Comment 14

[(no longer active)]

Looks nice!

Note that in your theme, the minimum width of the text in a tab seems to be larger than the default theme, which makes the issue less visible there.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Clearing old needinfo request. Please re-request or redirect if it is still pertinent.

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8642076 [details]
After tab overflow

Clearing old requests. Please re-request if this is still wanted.