Closed Bug 876041 Opened 7 years ago Closed 6 years ago

Granting access to microphone shows video icon next to address bar

Categories

(Firefox :: Site Permissions, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 30

People

(Reporter: mwobensmith, Assigned: florian)

References

Details

(Whiteboard: [getUserMedia][blocking-gum-])

Attachments

(8 files, 3 obsolete files)

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.
Summary: Granting access to microphone only shows only video icon next to address bar → Granting access to microphone shows video icon next to address bar
(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.
Component: WebRTC → General
Product: Core → Firefox
QA Contact: jsmith
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.
(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.
Whiteboard: [getUserMedia][blocking-gum-]
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.
Duplicate of this bug: 921004
Blocks: Talkilla
(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.
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)
File contains 9 microphone icons in Firefox toolbar style for windows/linux and SOX
(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 osx-webRTC-shareMicrophone-16@2x.png that seems identical to osx-webRTC-sharingMicrophone-16@2x.png
(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
32@2-default-osx.png

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/Toolbar-inverted@2x.png
/browser/themes/osx/Toolbar@2x.png
/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.
The file osx-webRTC-sharingMicrophone-16@2x.png in the zip file isn't correct. It has a size of 28 x 28 pixels. It should be 32 x 32.
Flags: needinfo?(jboriss)
Attached patch WIP Patch (obsolete) — Splinter Review
Assignee: nobody → florian
(In reply to Florian Quèze [:florian] [:flo] from comment #13)
> The file osx-webRTC-sharingMicrophone-16@2x.png in the zip file isn't
> correct. It has a size of 28 x 28 pixels. It should be 32 x 32.
Flags: needinfo?(jboriss)
Comment on attachment 8382636 [details]
32x32greyosx-webRTC-shareMicrophone-16@2x.png

It's the green file that I needed, and I've just noticed it was already on the bug (attachment 8381055 [details]).
Attachment #8382636 - Attachment is obsolete: true
Attachment #8381055 - Attachment filename: osx-webRTC-shareMicrophone-16@2x.png → osx-webRTC-sharingMicrophone-16@2x.p (green)ng
Attachment #8381055 - Attachment is obsolete: false
Attachment #8382454 - Flags: ui-review+
Attached image Screenshot (sharing)
Attachment #8382666 - Flags: ui-review+
Attached patch Patch v2 (including tests) (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=bf6c7526f085
Attachment #8382452 - Attachment is obsolete: true
Attachment #8382675 - Flags: review?(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.
Attachment #8382675 - Flags: review?(dolske) → review-
(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 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.
Attachment #8382675 - Flags: review- → review+
Fixed the Windows/Linux CSS.

https://hg.mozilla.org/integration/fx-team/rev/2030efeac518
Attachment #8382675 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/2030efeac518
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Keywords: verifyme
Pretty!

Verified Mac/Win/Linux Fx30, 2014-03-24.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Depends on: 1067444
Component: General → Device Permissions
Depends on: 1315228
You need to log in before you can comment on or make changes to this bug.