917367 - No notification visible indicator implemented for when getUserMedia audio is active

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Jason Smith [:jsmith]]

Build: Master 9/17/2013
Device: Inari

STR

1. Go to http://mozilla.github.io/webrtc-landing/gum_test.html
2. Go to audio
3. Grant permissions
4. Pull down the notification bar

Expected

The visible indicator should be present in the notification bar indicating that the mic is active at a target web origin.

Actual

The visible indicator isn't present in the notification bar.

Comment 1

[(inactive after 6/18) Alive Kuo [:alive]]

I think audio capture would trigger recording state changes -> then mozChromeEvent with event.detail.type = recording-status would be sent to system app.
SC^? However I recalled that I had seen the red icon on the statusbar during oslo workweek when we tested gum...

Comment 2

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Screen Shot 2013-09-19 at 7.11.55 PM.png

There is a red dot on notification bar while recording start already. What @jsmith means is that we should have an additional indicator on the notification window like the snapshot from UI spec I attached.

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

OK, I saw it in the document :/
Questions:
Could we know the recording time past in the chrome event?
Could we know who(page) is doing recording in the chrome event?

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

BTW, I tried to trace 'recording-device-events' in gecko, and it seems we are using this event for both audio and video recording. We need to distinguish.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

http://mxr.mozilla.org/mozilla-central/search?string=recording-device-events

Looks like we need to change lots of codes to implement "audio capture notification" + "video capture notification" because the original event only has RecordingStatus info.

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Indeed. From the UI spec we'll need add "type" and "origin" information in the mozChromeEvent. As for the time information, it should be the time that Gaia receive the mozChromeEvent. I think Gaia can get current time directly in javascript without read from mozChromeEvent.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Shih-Chiang Chien [:schien] from comment #6)
> Indeed. From the UI spec we'll need add "type" and "origin" information in
> the mozChromeEvent. As for the time information, it should be the time that
> Gaia receive the mozChromeEvent. I think Gaia can get current time directly
> in javascript without read from mozChromeEvent.

Uh, if I understand correctly, we need to know "recording time past". How do we know from javascript?

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Shih-Chiang Chien [:schien] from comment #6)
> Indeed. From the UI spec we'll need add "type" and "origin" information in
> the mozChromeEvent. As for the time information, it should be the time that
> Gaia receive the mozChromeEvent. I think Gaia can get current time directly
> in javascript without read from mozChromeEvent.

BTW We should use 'manifestURL' + 'pageURL' to identify a page, not origin.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Note: the desktop UI uses the recording-device-events notification as an indicator it needs to check if any of the active captures have been added or gone away:

function updateIndicators() {
  webrtcUI.showGlobalIndicator =
    MediaManagerService.activeMediaCaptureWindows.Count() > 0;

  let e = Services.wm.getEnumerator("navigator:browser");
  while (e.hasMoreElements())
    e.getNext().WebrtcIndicator.updateButton();

  for (let {browser: browser} of webrtcUI.activeStreams)
    showBrowserSpecificIndicator(browser);
}

showBrowserSpecificIndicator queries the capture state with each active capturing contentWindow for audio and video capture.  This includes any embedded iframes using webrtc, etc, and lets us have a list of active what that window is capturing (which you may not need).

So I don't think any API changes are needed to get the info the UI needs.

See browser/modules/webrtcUI.jsm

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

Thanks for the info.
It looks like MediaManagerService.activeMediaCaptureWindows has everything we need.

Maybe we could parse these info in shell.js and pass it to system?

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

In shell.js, I found MediaManagerService.activeMediaCaptureWindows.Count() is 0 even when audio capture is running :/

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

activeMediaCaptureWindows should work; it's used on desktop per above to enable/disable the active-capture-window dropdown

Perhaps it's something in shell.js?  Perhaps it's running in the wrong process?  (I know nothing about shell.js on B2G)

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Randell Jesup [:jesup] from comment #12)
> activeMediaCaptureWindows should work; it's used on desktop per above to
> enable/disable the active-capture-window dropdown
> 
> Perhaps it's something in shell.js?  Perhaps it's running in the wrong
> process?  (I know nothing about shell.js on B2G)

Yes, because shell.js running in chrome process but the app having media manager service running in content process.
We need to send the recording-device-events appending with mActiveWindow, not only recording status.

http://mxr.mozilla.org/mozilla-central/source/dom/ipc/PContent.ipdl#461

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

I think I need at least manifestURL + pageURL to identify the app/page.
manifestURL is browserElement.getAttribute('mozapp').

Comment 15

[bhavana bajaj [:bajaj]]

needinfo : Ivan here to help the decision on the bug.

From  the read of it and my discussion with :jsmith it looks like a feature miss, but we may have privacy concern's if this is left um-implemented.

So, Ivan how do we want to move forward wrt 1.2 here ?

Comment 16

[Jason Smith [:jsmith]]

Mentioned this in person to rel management, but main sticking point for blocking on this is that a user needs to have a clear understanding of what device is active. Right now, the recording indicator is used interchangeably with getUserMedia and the camera app. That means that a user right now can distinguish that recording is active, but not understand which device (microphone or camera) is doing the recording. That's a privacy concern if the user cannot distinguish that.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

Extract the gecko part to bug 926289.
Change component of this bug to gaia::system

Comment 18

[(inactive after 6/18) Alive Kuo [:alive]]

Patryk, who could provide the notification recording icon asset?

Comment 19

[Ivan Tsay (:ITsay)]

Attachment: gUM_permission_2013-10-14 3.00.39 PM.png

Comment 20

[Ivan Tsay (:ITsay)]

(In reply to bhavana bajaj [:bajaj] from comment #15)
> needinfo : Ivan here to help the decision on the bug.
> 
> From  the read of it and my discussion with :jsmith it looks like a feature
> miss, but we may have privacy concern's if this is left um-implemented.
> 
> So, Ivan how do we want to move forward wrt 1.2 here ?

Hi Bhavana,

This one should not be the feature miss but a privacy issue on gUM. While planning this Microphone API gUM, I and CJ committed to finish A3 of UX design on gUM (please see attached image). However, in this bug, it require to finish all up to A5. After discussing it with Alive, I can still plus this one and we will try to finish it by Oct. 28.

Comment 21

[Ivan Tsay (:ITsay)]

Add keyword for tracking

Comment 22

[Jason Smith [:jsmith]]

(In reply to Ivan Tsay (:ITsay) from comment #20)
> (In reply to bhavana bajaj [:bajaj] from comment #15)
> > needinfo : Ivan here to help the decision on the bug.
> > 
> > From  the read of it and my discussion with :jsmith it looks like a feature
> > miss, but we may have privacy concern's if this is left um-implemented.
> > 
> > So, Ivan how do we want to move forward wrt 1.2 here ?
> 
> Hi Bhavana,
> 
> This one should not be the feature miss but a privacy issue on gUM. While
> planning this Microphone API gUM, I and CJ committed to finish A3 of UX
> design on gUM (please see attached image). However, in this bug, it require
> to finish all up to A5. After discussing it with Alive, I can still plus
> this one and we will try to finish it by Oct. 28.

It's technically a feature miss if the full feature didn't land in time for feature complete. This was part of the core spec for the feature which should have been implemented before feature complete. This is particularly important because implementations like these will require strings, so the later this is implemented, the more localization risk becomes apparent.

Comment 23

[Patryk Adamczyk [:patryk] UX]

Alive, we added a recording tone with bug, please reference it.
https://bugzilla.mozilla.org/show_bug.cgi?id=896874

Comment 24

[[e-mail]]

Patryk, we need the real image of 'red dot'(?) shown in spec (Left icon for Notification message)
https://bug917367.bugzilla.mozilla.org/attachment.cgi?id=807149

Can you help provide the icon assets?

Comment 25

[[e-mail]]

taken

Comment 26

[Patryk Adamczyk [:patryk] UX]

Peter La should be able to provide the icon.

Comment 27

[[e-mail]]

Currently gUM blocked by bug 926746. it means we can't test on real device yet.
But I'd start from UI-related work first.

Comment 28

[[e-mail]]

Attachment: with notification style

WIP: take notification style and reuse gUM permission list icons

Comment 29

[[e-mail]]

gUM permission list icons are 64x64, still expect replaced by smaller icons

Comment 30

[[e-mail]]

Attachment: pull request redirect to github

WIP

alive, could you kindly check if it's the expect way to implement this feature?
will add unittest if its in right path.

Comment 31

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 821444 [details] [review]
pull request redirect to github

See github.

Comment 32

[[e-mail]]

Peter, please provide Video(camera)/Media(recorder) icons as well, so we not need to do it again in 1.3.

Comment 33

[[e-mail]]

PR is ready but need add unittest, we'll add 3 strings into system locales, add l10n team to check early

microphone-is-on=Microphone is on
camera-is-on=Camera is on
media-is-on=Camera and Microphone are on

Comment 34

[Axel Hecht [:Pike]]

I would expect that in particular the media-is-on string exceeds a single line, can't tell from the screenshots if that works, though.

Note, needinfo on me for string changes isn't necessary. You need to coordinate with release management on when you need to be done. Our team is focusing on getting the strings that are landed out to localizers.

Comment 35

[[e-mail]]

thanks, BTW I've checked media-is-on string just fit the single line.

Comment 36

[Axel Hecht [:Pike]]

English:
Camera and Microphone are on

German:
Kamera und Mikrofon eingeschaltet

You should expect that localizations are longer than English.

Comment 37

[Axel Hecht [:Pike]]

French just came in on irc:

La caméra et le microphone sont en marche

Comment 38

[[e-mail]]

patryk, would you like to suggest some shorter 'media-is-on' string? thanks.

While following the gUM UI spec, we also find many inconsistency places such as the gUM icons & strings are vary, and some dialogs still have 'remember' toggle button, which should not be shown there.
Hope we could get an more accurate version.

Comment 39

[Patryk Adamczyk [:patryk] UX]

I have a few suggestions.

1. Microphone seems long winded, its often abbreviated to "MIC", so it could read like this:
- Camera and Mic are on

2. I would reduce the words and emphasis the ON / OFF
- Camera & Mic: ON

Fred, who wrote the gUM spec? Can you send it over and we can scrub the inconsistencies.

Comment 40

[Randell Jesup [:jesup] (needinfo me)]

W3 Media Capture Task Force
http://dev.w3.org/2011/webrtc/editor/getusermedia.html

Comment 41

[Jason Smith [:jsmith]]

(In reply to Patryk Adamczyk [:patryk] UX from comment #39)
> I have a few suggestions.
> 
> 1. Microphone seems long winded, its often abbreviated to "MIC", so it could
> read like this:
> - Camera and Mic are on
> 
> 2. I would reduce the words and emphasis the ON / OFF
> - Camera & Mic: ON
> 
> Fred, who wrote the gUM spec? Can you send it over and we can scrub the
> inconsistencies.

Point of clarification - you mean the W3C spec jesup cites in comment 40 or the UX spec?

Comment 42

[[e-mail]]

Attachment: gUM_WebRTC_Permissions_2013-08-12.pdf

Patryk, 

I got the UI spec named gUM/WebRTC permissions authored by Esther Leong & Jacqueline Savory, last update date is 8/12/2013

Comment 43

[[e-mail]]

Comment on attachment 821444 [details] [review]
pull request redirect to github

added the unittest and passed travis test

though still need gecko bug resolved to test on real device, and need update with new images

Comment 44

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

I can provide an image that works around the bug 926746 for your to test.

Comment 45

[[e-mail]]

Thanks SC, I've test in real device and works well. (with bug 919927 WIP in master)

Comment 46

[Peter La]

Attachment: ScreenMockup.png

Hi Fred,

I'm proposing that we use a red dot to cover every case because it's visually stronger and more attention grabbing than using mic and video camera iconography.

The fact that we have l10n text that says 'Microphone is On' or 'Video Camera is On' means we don't need to explicitly show it in the icon.  The combination of the text and red dot is strong.

Attached is a screenshot mocking this up.

Comment 47

[Peter La]

Attachment: icon_notificationtray_microphoneon.png

Here is the icon asset for the red dot.

Comment 48

[[e-mail]]

Hi peter, thanks for provide the icon. 
I think we need 1.5x, 2x icons as well.

Is there any difference between mic/camera/both icons? Or all of them use the same one?

Comment 49

[Jason Smith [:jsmith]]

(In reply to Peter La from comment #46)
> Created attachment 825928 [details]
> ScreenMockup.png
> 
> Hi Fred,
> 
> I'm proposing that we use a red dot to cover every case because it's
> visually stronger and more attention grabbing than using mic and video
> camera iconography.
> 
> The fact that we have l10n text that says 'Microphone is On' or 'Video
> Camera is On' means we don't need to explicitly show it in the icon.  The
> combination of the text and red dot is strong.
> 
> Attached is a screenshot mocking this up.

No that's not right - that doesn't meet the privacy requirements of the feature. There's a critical need that the user has to have always a clear indication of what recording is active - camera, mic, or both. That means being as explicit as possible across the UX as much as possible. Hiding information anywhere means we risk reducing user knowledge of what's actually active, so we risk cases where the user might think say, the camera is active when the mic is actually active.

We already know that the explicit icon usage in notification indicators worked successfully on Firefox for Android, so I don't really think what's proposed here is the right direction to go to protect against user privacy.

needinfo on Paul for his opinion here as well from the security perspective

Comment 50

[Paul Theriault [:pauljt] (no longer reading bugmail)]

> needinfo on Paul for his opinion here as well from the security perspective

Hmmm I don't know if I feel strongly either way, sorry!  On one hand I sort of agree with the red dot being visually stronger, but then is this at the expense of the attention to other items ? Wont users just ignore any icons that are not red?

Jason's point about losing information seems valid too though, and it seems like a bad idea to have vastly different iconography on Firefox OS vs Firefox Mobile vs Firefox Desktop.

However given where we are/were with audio notification, from my perspective I could live with just the red dot for now (to me the behavior of when and where it is shown is much more important than the actual icon), but I would like us to revisit our approach to security UI as part of Haida effort so that we get some kind of cohesive approach. This is especially in the case of persistent notifications for active permissions, which will only become more important as we expose more sensitive APIs.

Comment 51

[Peter La]

Attachment: Icon Concepts

Hi Jason/Paul,

Thanks for sharing that information.  I've attached 3 icon concepts that show more explicitly each case.  I'd love to get your feedback before we finalize tomorrow.

Fred, I will be providing all of the icons once we finalize, and yes, they will be three distinct icons.

Comment 52

[Jason Smith [:jsmith]]

(In reply to Peter La from comment #51)
> Created attachment 827174 [details]
> Icon Concepts
> 
> Hi Jason/Paul,
> 
> Thanks for sharing that information.  I've attached 3 icon concepts that
> show more explicitly each case.  I'd love to get your feedback before we
> finalize tomorrow.

I like the direction of the three concepts. I like the red icon with white mic/camera icons the best, as it seems to keep the red icon concept for recording & is readable to provide indication that the camera, mic, or both are active.

Comment 53

[Peter La]

(In reply to Jason Smith [:jsmith] from comment #52)
> (In reply to Peter La from comment #51)
> > Created attachment 827174 [details]
> > Icon Concepts
> > 
> > Hi Jason/Paul,
> > 
> > Thanks for sharing that information.  I've attached 3 icon concepts that
> > show more explicitly each case.  I'd love to get your feedback before we
> > finalize tomorrow.
> 
> I like the direction of the three concepts. I like the red icon with white
> mic/camera icons the best, as it seems to keep the red icon concept for
> recording & is readable to provide indication that the camera, mic, or both
> are active.

Great, that's our consensus as well.  The red circle + white icon provides the clearest read.

Fred, Amy Lee will be exporting and attaching the icons @1x and @1.5x size to this bug shortly.

Comment 54

[Amy Lee [:amylee] UX]

Attachment: Notification_Icons.zip

Hi Fred, 

Attached are the icons in @1 and @1.5 scale. Let me know if you need anything else.

Comment 55

[[e-mail]]

merged to gaia-master https://github.com/mozilla-b2g/gaia/commit/00d6e5be8db6851dd202598eddd3d09017e29f46

thanks!

Comment 56

[[e-mail]]

Amy, thanks for help. We may need a separate permission prompt window icons to present video&Mic case.

currently we use the same icon as video

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/permission_manager/images/PermissionsDialogIcons_VideoRecorder.png

please attach the icon in bug 935342

Comment 57

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 00d6e5be8db6851dd202598eddd3d09017e29f46 to:
v1.2: e18f19c51576a2876fae22be2e442bddac743caa

Comment 58

[Jason Smith [:jsmith]]

Comment on attachment 821444 [details] [review]
pull request redirect to github

This really needed a UI review before this landed. I'm adding the ui review flag for Jacqueline to take a look at this to see if what landed aligns with the gUM UX spec.

Comment 59

[Jason Smith [:jsmith]]

Just finished testing this - see the dependencies of bug 918876 recently filed for bugs that were found while testing this. The only bug that was found during this testing that likely is going to block 1.2 is bug 940045.

Comment 60

[[e-mail]]

Jason, feel free to cc me if it's a gaia related confidential bug

Comment 61

[Jason Smith [:jsmith]]

(In reply to Fred Lin [:gasolin] from comment #60)
> Jason, feel free to cc me if it's a gaia related confidential bug

I think it's gecko related, but if it ends up becoming gaia related, I'll let you know.

Comment 62

[Jason Smith [:jsmith]]

Comment on attachment 821444 [details] [review]
pull request redirect to github

I actually ended up doing a QA pass that looked at some of the UX implications, so I don't think we need to wait on a review at this point. But feel free to look at this point after the fact if you like.