876041 - Granting access to microphone shows video icon next to address bar

Status: VERIFIED FIXED

(Firefox :: Site Permissions, defect)

Description

[Matt Wobensmith [:mwobensmith][:matt:]]

Attachment: Click link, share microphone and observe icon next to address bar

When opting into microphone sharing, wrong icon is displayed.

Expected something resembling a microphone, but it displays the generic video icon. 

This can be misleading to the user.

Comment 1

[Jason Smith [:jsmith]]

(In reply to Matt Wobensmith from comment #0)
> Created attachment 754015 [details]
> Click link, share microphone and observe icon next to address bar
> 
> When opting into microphone sharing, wrong icon is displayed.

Ehh...technically Chrome actually uses this icon as well. We could do this, but it's quite minor IMO. It could also get more puzzling if you end up having to switch icons in case multiple gUM requests come in to increase audio --> video/audio. So I'm not sure this bug has value to fix. This up to UX, however.

> 
> Expected something resembling a microphone, but it displays the generic
> video icon. 
> 
> This can be misleading to the user.

The expectation of a user in this case that they need to understand that the tab is presently using media content from gUM (visible icon). That's already pretty well understood. We aren't doing different than Chrome here either.

Comment 2

[Matt Wobensmith [:mwobensmith][:matt:]]

I mention it because I had a conversation with Boriss, who feels that there might already be a microphone icon for this purpose that isn't being used. Correct me if I'm wrong.

Comment 3

[Jason Smith [:jsmith]]

(In reply to Matt Wobensmith from comment #2)
> I mention it because I had a conversation with Boriss, who feels that there
> might already be a microphone icon for this purpose that isn't being used.
> Correct me if I'm wrong.

I don't have any knowledge if we had a microphone icon or not.

Comment 4

[Dão Gottwald [:dao]]

This is supposed to be a generic icon that can be used for all kinds of requests (mic, camera, mic+camera). I agree that, unfortunately, it more or less just resembles a camera.

Comment 6

[Florian Quèze [:florian]]

(In reply to Jason Smith [:jsmith] from comment #3)
> (In reply to Matt Wobensmith from comment #2)
> > I mention it because I had a conversation with Boriss, who feels that there
> > might already be a microphone icon for this purpose that isn't being used.
> > Correct me if I'm wrong.
> 
> I don't have any knowledge if we had a microphone icon or not.

I just found that there are microphone icons attached in obsolete attachments of bug 798336.

Comment 7

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Zip file: 12 images: webRTC shareMicrophone icons, windows/osx/linux

Zip file contains 12 images of microphones like the 12 images of video cameras symmetrical to images at lines 3-14 of http://mxr.mozilla.org/mozilla-central/find?text=&string=webRTC (as of Feb 2014)

Comment 8

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Zip file: 9 images: Firefox toolbar icons icons, windows/osx

File contains 9 microphone icons in Firefox toolbar style for windows/linux and SOX

Comment 9

[Florian Quèze [:florian]]

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #7)
> Created attachment 8381002 [details]
> Zip file: 12 images: webRTC shareMicrophone icons, windows/osx/linux
> 
> Zip file contains 12 images of microphones like the 12 images of video
> cameras symmetrical to images at lines 3-14 of
> http://mxr.mozilla.org/mozilla-central/find?text=&string=webRTC (as of Feb
> 2014)

Looks good, except [e-mail] that seems identical to [e-mail]

Comment 10

[Florian Quèze [:florian]]

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #8)
> Created attachment 8381003 [details]
> Zip file: 9 images: Firefox toolbar icons icons, windows/osx
> 
> File contains 9 microphone icons in Firefox toolbar style for windows/linux
> and SOX

The 9 files included are:
18-clicked-osx.png
18-clicked-win-inverted.png
18-clicked-win.png
18-default-osx.png
18-default-win.png
32-clicked-osx.png
32-default-osx.png
32-default-win.png
[e-mail]

I don't think I need the 32px versions (IIRC the WebRTC icon can't be customized away), but I'm afraid there are more than 5 versions of the 18px icons for the camera. The camera icon appears in:
/browser/themes/linux/Toolbar-inverted.png (same file as the Windows version)
/browser/themes/linux/Toolbar-small.png (I don't really know how/where this is used)
/browser/themes/linux/Toolbar.png (same file as the Windows version)
/browser/themes/osx/Toolbar-inverted.png
/browser/themes/osx/Toolbar.png
/browser/themes/osx/[e-mail]
/browser/themes/osx/[e-mail]
/browser/themes/windows/Toolbar-inverted.png
/browser/themes/windows/Toolbar.png
/browser/themes/windows/Toolbar-aero.png
/browser/themes/windows/Toolbar-lunaSilver.png

mxr query to get links: http://mxr.mozilla.org/mozilla-central/find?string=toolbar&tree=mozilla-central&hint=browser%2Fthemes

So if we look only at the 18px sized icon, and ignore Linux, we already need 8 versions of the icon, each in clicked and non clicked state.

Given how many files this involves, I think we should deal with the toolbar icon in a separate bug to keep the current bug focused.

Comment 11

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Icon: [e-mail]

Comment 12

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Icon: [e-mail] (grey)

Comment 13

[Florian Quèze [:florian]]

The file [e-mail] in the zip file isn't correct. It has a size of 28 x 28 pixels. It should be 32 x 32.

Comment 14

[Florian Quèze [:florian]]

Attachment: WIP Patch

Comment 15

[Florian Quèze [:florian]]

Attachment: Screenshot with the WIP applied

Comment 16

[Jennifer Morrow [:Boriss] (UX)]

Attachment: [e-mail]

(In reply to Florian Quèze [:florian] [:flo] from comment #13)
> The file [e-mail] in the zip file isn't
> correct. It has a size of 28 x 28 pixels. It should be 32 x 32.

Comment 17

[Florian Quèze [:florian]]

Comment on attachment 8382636 [details]
[e-mail]

It's the green file that I needed, and I've just noticed it was already on the bug (attachment 8381055 [details]).

Comment 18

[Florian Quèze [:florian]]

Attachment: Screenshot (sharing)

Comment 19

[Florian Quèze [:florian]]

Attachment: Patch v2 (including tests)

https://tbpl.mozilla.org/?tree=Try&rev=bf6c7526f085

Comment 20

[Justin Dolske [:Dolske]]

Comment on attachment 8382675 [details] [diff] [review]
Patch v2 (including tests)

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

What's the intended UX if the user shares only the camera, and the later (for the same page) additionally shares the microphone (or vice-versa)? Ditto for if a prompt for one is currently pending, and then a request for the user other is made?

It seems like this should all just be one prompt and anchor, which gets tweaked depending upon the exact type of request. Otherwise you risk getting both prompts concurrently, and that seems unwanted. In the |shown| handler you could just set an attribute or two (if needed), and use CSS to control which icon is actually used. For example:

  #webRTC-shareDevices-notification-icon[device=cam] { /* [device=camAndMic] too? */
    list-style-image: url(chrome://browser/skin/webRTC-shareDevice-16.png);
  }
  #webRTC-shareDevices-notification-icon[device=mic] {
    list-style-image: url(chrome://browser/skin/webRTC-shareMicrophone-16.png);
  }

  ...

  if (aTopic == "shown") {
    ...
    let anchorNode = ...
    anchorNode.setAttribute("device", requestType == "Microphone" ? "mic" : "cam"))
    ...
  }

Or am I crazy?

::: browser/themes/osx/browser.css
@@ +3455,5 @@
>  
> +.webRTC-shareMicrophone-notification-icon,
> +#webRTC-shareMicrophone-notification-icon {
> +  list-style-image: url(chrome://browser/skin/webRTC-shareMicrophone-16.png);
> +}

You've only made these additions to the OSX browser.css, you'll need to do it for Windows and Linux too.

Comment 21

[Justin Dolske [:Dolske]]

(In reply to Justin Dolske [:Dolske] from comment #20)

> Or am I crazy?

*cough*

I see now that PopupNotifications handles this, since the notification's ID is the same and it's just shown against different attachment icons. I think this is the first place we actually _do_ that, although that's what PopupNotifications says it supports. The Try build acts as I'd expect.

Comment 22

[Justin Dolske [:Dolske]]

Comment on attachment 8382675 [details] [diff] [review]
Patch v2 (including tests)

Technically still a r-, since you need to fix the Windows/Linux CSS, but since that's that now the only issue I trust you will make the obvious fix before checkin.

Comment 23

[Florian Quèze [:florian]]

Attachment: Patch v3 (as checked in)

Fixed the Windows/Linux CSS.

https://hg.mozilla.org/integration/fx-team/rev/2030efeac518

Comment 24

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/2030efeac518

Comment 25

[Matt Wobensmith [:mwobensmith][:matt:]]

Pretty!

Verified Mac/Win/Linux Fx30, 2014-03-24.