913896 - Display audio (microphone) permission in permission acquisition prompt

Status: VERIFIED FIXED

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

Description

[[e-mail]]

To support Display camera/ microphone permission acquisition prompt,

Permission Manager need to show the available device list get from gecko, with single select option box near it.

Comment 1

[[e-mail]]

this bug will focus on audio device list.


UX please provide 

1. icon for Microphone Permissions
2. 'Microphone is on' image for top bar
3. 'Microphone is on' icon for navigation item

Comment 2

[[e-mail]]

FYI, at the end there will be 3 kinds of getUserMedia permissions:

1. Microphone only
2. Camera only
3. Camera + Microphone

Comment 3

[[e-mail]]

Current confirm button strings are "Don't allow" and "Allow".

In spec p15, said the button strings are 'Don’t Share' and 'Share'.

Should 'Share/Don't share' be applied to all permissions, or used only for microphone, camera inputs?

Comment 4

[Stephany Wilkes]

Matej, please advise on the expected standard strings here: do we use Allow and Don't allow or Share and Don't share? Thanks!

Comment 5

[Matej Novak [:matej]]

I think it's always better for the button text to be as clear as possible. In this case, I prefer "Share" and "Don't share" since the user will know what they're doing. Thanks.

Comment 6

[[e-mail]]

Matej, you mean only use "Share" and "Don't share" for microphone, camera inputs, 
but not for geo location, fm radio...etc, right?

Comment 7

[Matej Novak [:matej]]

(In reply to Fred Lin [:gasolin] from comment #6)
> Matej, you mean only use "Share" and "Don't share" for microphone, camera
> inputs, 
> but not for geo location, fm radio...etc, right?

I think if the action is sharing, it should always say so on the buttons. Why wouldn't we be consistent?

Comment 8

[[e-mail]]

Matej, thanks for feedback!
So I'd replace "Don't allow" and "Allow" to "Share" and "Don't share" for button text.

Comment 9

[[e-mail]]

Eric, could you help on provide images described in Comment 1 ?

Comment 10

[Eric Rescorla (:ekr)]

Fred: you mean what does it look like on Desktop?

Comment 11

[[e-mail]]

Eric,

I found each kinds of permission has it own icon
https://github.com/mozilla-b2g/gaia/tree/master/apps/system/style/permission_manager/images

So basically I'd request have audio icon to fulfill this issue (better to provide video/media icons as well)

thanks!

Comment 12

[Stephany Wilkes]

Just checking on the UX need here: Are new icons needed, or do you need the existing icons (referenced above) in different sizes, etc?

Comment 13

[[e-mail]]

new icons needed

Comment 14

[[e-mail]]

WIP with default choice (and correspondent test cases) https://github.com/mozilla-b2g/gaia/pull/12098

@schien you can check if it works fine with your patch

Comment 15

[Stephany Wilkes]

Hi Fred - sorry, I'm still not clear on the icon request here. Please let me know which exact icons are needed (from comment #11), and how they need to be different from existing icons. I've also flagged Eric Pang to see if he can advise on details here.

Comment 16

[[e-mail]]

sorry for unclear message. we need icons for permission prompt screen. 

* microphone capture
* camera capture
* microphone & camera capture

as shown in spec p11(video)/p13(microphone), but they might be in 'blue' color to align other permission icons

Comment 17

[[e-mail]]

Attachment: screenshot_need_icon

attach screenshot, we need new icon to replace this one described in spec p11

Comment 18

[[e-mail]]

Attachment: pull request redirect to github

current lack correspondent icons, but due to schedule, I'd like send for review and replace icons before landing.

Comment 19

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

Comment on attachment 803591 [details]
pull request redirect to github

r+ if github comments are addressed.

Comment 20

[Eric Pang [:epang] UX]

Attachment: Permission Icons.zip

Permissions Icons attached

Comment 21

[[e-mail]]

Thanks you eric for icons. 

Updated the PR and will land after some integration test.

Comment 22

[Stephany Wilkes]

Thank you Eric! :)

Comment 23

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

Integration test fails because following JS error is shown after tapping the "Share" button, and Gecko doesn't receive mozContentEvent with correct detail attribute.

E/GeckoConsole(  688): [JavaScript Error: "this.showPermissionPrompt(...) is undefined" {file: "app://system.gaiamobile.org/js/permission_manager.js" line: 258}]

Comment 24

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

I still don't know why it's undefined from interpreting javascript in my brain. Need device to do more test..

Comment 25

[[e-mail]]

alive, I missed to bind(this) while calling requestPermission in L142 ?

SC, thanks for the integration test!

normal permission Event format (what gecko will receive) is

`
{
  id: id,
  type: '....allow',
  remember: true
}
`
I think the format is different from what we got from gecko. (no detail attribute)

and current return ContentEvent after this patch will be

`
{
  id: id,
  type: '....allow',
  remember: true
  // media only
  choice: {
    "video-capture": "front",
    "audio-capture": ""
  }
}
`

where should 'detail' attribute come with?

Comment 26

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

'detail' attribute is the father attribute which hold the object passed to the initCustomEvent.[1] You need to use |initCustomEvent| instead of |new CustomeEvent| in your patch.[2]

[1] https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent
[2] https://github.com/mozilla-b2g/gaia/pull/12098/files#L1R161

Comment 27

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

We need to complete this today, stealing.

Comment 28

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/12193

Part 2 - Fix custom event malfunction.

Comment 29

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

master

https://github.com/mozilla-b2g/gaia/commit/26f30419ad2ba17383ad81cd2ba85e889bc2262b

Comment 31

[Jason Smith [:jsmith]]

Verified through initial test pass of gUM. See dependencies on bug 894848 for potential followups.