Closed Bug 1189759 Opened 4 years ago Closed 4 years ago

Add tab sound icon to subject tab in tabs list drop-down

Categories

(Firefox :: Tabbed Browser, enhancement)

enhancement
Not set

Tracking

()

VERIFIED FIXED
Firefox 42
Tracking Status
firefox42 --- verified

People

(Reporter: fehe, Assigned: ehsan)

References

Details

Attachments

(6 files, 2 obsolete files)

For people with a large number of tabs per window, it would be great if the following enhancement could be made:

If reasonably easy to do, it would be great if the tab sound icon could also appear in the tabs list drop-down, for the respective tab playing audio (preferably at the end of the tab name)

For this particular case, no muting functionality is required -- just the icon -- so someone can quickly locate, switch to the tab and deal with it.
This is a reasonable feature request, I think, but it should not block shipping the current audio indicators.
No longer blocks: tab-sound-icon
Depends on: 486262
Assignee: nobody → ehsan
Attached image Screenshot (obsolete) —
I'd like to get a review on the UI before finishing up the patch and make it ready for review.  (I need to do some more testing on Windows and Linux before the patch is ready for review...)
Attachment #8642068 - Flags: ui-review?(shorlander)
Attachment #8642068 - Flags: ui-review?(madhava)
Note that these menu items will behave like normal items, which means that the sound playing icon cannot be clicked on individually.  Activating the menu will focus the noisy tab as usual.
(In reply to :Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #1)
> This is a reasonable feature request, I think, but it should not block
> shipping the current audio indicators.

Actually, thinking about this more, this may be a nice way to address bug 1186633, or at least part of a solution for that, so this should block.
See Also: → 1186633
Comment on attachment 8642067 [details] [diff] [review]
Add sound playing and muted icons to the all tabs menu; r=jaws

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

Drive-by feedback. Most of the patch looks fine.

::: toolkit/content/widgets/menu.xml
@@ +222,5 @@
>        </xul:hbox>
>      </content>
>    </binding>
>  
> +  <binding id="menuitem-iconic-right" extends="chrome://global/content/bindings/menu.xml#menuitem">

Since this new binding allows icons on both the left (via 'image' attribute) and right (via 'rightimage' attribute), I think this name should be "menuitem-iconic-both", "menuitem-iconic-leftright", or "menuitem-iconic-startend".

Further, can we adjust the attributes to "startimage" and "endimage" instead of "image" and "rightimage", respectively? The downside to this is breaking the consistency of specifying "image" for the left image on these bindings.
Attachment #8642067 - Flags: feedback+
Comment on attachment 8642068 [details]
Screenshot

LGTM. Is that the smaller icon? Probably have enough space there for the bigger one.
Attachment #8642068 - Flags: ui-review?(shorlander) → ui-review+
(In reply to Stephen Horlander [:shorlander] from comment #7)
> Comment on attachment 8642068 [details]
> Screenshot
> 
> LGTM. Is that the smaller icon? Probably have enough space there for the
> bigger one.

Yeah it is the smaller icon.  I'll see if the bigger one fits there (I think it should.)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> Since this new binding allows icons on both the left (via 'image' attribute)
> and right (via 'rightimage' attribute), I think this name should be
> "menuitem-iconic-both", "menuitem-iconic-leftright", or
> "menuitem-iconic-startend".

OK.

> Further, can we adjust the attributes to "startimage" and "endimage" instead
> of "image" and "rightimage", respectively? The downside to this is breaking
> the consistency of specifying "image" for the left image on these bindings.

I'm fine with "endimage" but I don't think we have a good reason to change "startimage", since that might break add-on code, and the majority of menu items only have one image, so startimage for them is not even a good name.

Do you agree?
Flags: needinfo?(jaws)
Ok, I'm fine with "image" and "endimage" being that "image" would then imply it is not at the end and therefore possibly at the start.
Flags: needinfo?(jaws)
Attached image Final screenshot (OSX)
Attachment #8642068 - Attachment is obsolete: true
Attachment #8642068 - Flags: ui-review?(madhava)
Attachment #8643478 - Flags: review?(jaws)
Attachment #8643479 - Flags: review?(jaws)
Attachment #8643478 - Flags: review?(jaws) → review+
Attachment #8643479 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/dfe28c09bed9
https://hg.mozilla.org/mozilla-central/rev/de03e403128c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Thanks for implementing this. Working without issues.
Depends on: 1192598
Depends on: 1204845
Verified fixed using latest Aurora 42.0a2 (buildID: 20150917004025).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.