Closed Bug 978660 Opened 10 years ago Closed 10 years ago

Setting "GRANT" for audio-capture or video-capture permissions is equivalent to DENY

Categories

(Core :: WebRTC, defect)

Product:
Core
Component:
WebRTC
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
2.0 S3 (6june)
Project Flags:
feature-b2g 2.0

People

(Reporter: pauljt, Assigned: schien)

References

Details

(Whiteboard: [p=2][webRTC]ft:loop)

Attachments

(1 file, 3 obsolete files)

support permission granted/deny for gUM permission, v2
14.41 KB, patch
fabrice
: review+
Details | Diff | Splinter Review 
So I guess this is a symptom of a rushed feature due to changed timelines, but video-capture and audio-capture show up in the settings app and can be modified (as they should). However if you set either permission to "GRANT" instead of "ASK" then all calls to getUserMedia just fail with permission denied.

STR:
1. Install an app that uses webrtc (having either audio-capture or video-capture) in the manifest
2. Open the "App permissions" section of the setting app, and set the permission to "GRANT" instead of the default ("ASK")
3. Attempt to use webrtc in the app.

Actual:
Permission denied error when attempting to call navigator.getUserMedia

Expected result:
Well I'm not sure, since the behavior doesn't follow the specs yet (i.e. we don't support remembering permissions). I was expecting it just to behave the same as ask though. Might be easier and less risky to finish support for remembering the permission choice though.


Probably not the end of the world if this isn't fixed, but its a bit confusing for the user.
We should probably backout bug 945111 then if the behavior isn't right. No point of exposing the UI if it doesn't work.
Blocks: b2g-getusermedia, 945111
No longer blocks: 923361
Whiteboard: [ucid:WebRTC7, 1.4, ft:multimedia-platform][NPOTB]
SC -- Can you tell us how much effort it would take to make "GRANT" have the same behavior as "ASK"?  (In other words, we want the "GRANT" to prompt permission just as "ASK" does since we don't yet support remembering permissions, except for privileged apps.)  Thanks.
Flags: needinfo?(schien)
(In reply to Maire Reavy [:mreavy] from comment #2)
> SC -- Can you tell us how much effort it would take to make "GRANT" have the
> same behavior as "ASK"?  (In other words, we want the "GRANT" to prompt
> permission just as "ASK" does since we don't yet support remembering
> permissions, except for privileged apps.)  Thanks.

That would be a very bad idea - that's misleading UX. If you don't want support a use case, then don't show the UI.

I think the best option here is to backout bug 945111 and file a separate bug to do underlying work for remembering my decision for apps we can trust with that decision.
(In reply to Jason Smith [:jsmith] from comment #3)
> 
> I think the best option here is to backout bug 945111 and file a separate 
> bug to do underlying work for remembering my decision for apps we can trust
> with that decision.

It may well be the best option.  What functionality do we lose if we back it out?  

(Note to all: "remember my decision" was never in scope for the first release of gUM, other than for privileged apps.)
(In reply to Maire Reavy [:mreavy] from comment #4)
> (In reply to Jason Smith [:jsmith] from comment #3)
> > 
> > I think the best option here is to backout bug 945111 and file a separate 
> > bug to do underlying work for remembering my decision for apps we can trust
> > with that decision.
> 
> It may well be the best option.  What functionality do we lose if we back it
> out?  

We would probably lose the ability to do DENY on all audio-capture & video-capture requests from the settings app. However, we haven't ever shipped with this feature (it's lived on trunk in 1.4), so there isn't a end-user facing regression here.
Attached patch always-prompt.patch (obsolete) — — Splinter Review 
Always prompt for media permission request if not denied. The behavior will be:

  ASK, GRANT, UNKNOWN: prompt to user
  DENY: deny without prompt

If we don't want user to denial the use of microphone/camera, I'll suggest to remove microphone/camera from App permission menu directly.
Flags: needinfo?(schien)
Attachment #8384517 - Flags: feedback?(fabrice)
Comment on attachment 8384517 [details] [diff] [review]
always-prompt.patch

This is not the right fix for this bug nor does this make sense to implement. The fix here should focus on ensuring we're exposing the right UX here based on the app type in Gaia for audio & video capture permissions.

Separate note for the gecko fix - what we should really be doing here is:

* If the app security level (privileged or certified) is going to permit remember my choice, then follow the normal remember my choice workflow we do for other PROMPT_ACTION WebAPIs in apps

* If app security level (hosted or web) is not going to permit remember my choice, then we should error out if we somehow get into a state where a remember my choice request is made with an audio-capture or video-capture request, as that should not be possible.
Attachment #8384517 - Flags: feedback-
Comment on attachment 8384517 [details] [diff] [review]
always-prompt.patch

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

Can anyone point me to why we don't support remembering the user choice except in some app type? that's new to me.
Attachment #8384517 - Flags: feedback?(fabrice)
Fixed via backout of bug 945111.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Jason Smith [:jsmith] from comment #9)
> Fixed via backout of bug 945111.

This does not 'fix'this issue, it just hides it. Backing out 945111 just creates other problems imho. I'm going to take this to email since there are a lot of moving parts here.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Fabrice Desré [:fabrice] from comment #8)
> Comment on attachment 8384517 [details] [diff] [review]
> always-prompt.patch
> 
> Review of attachment 8384517 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can anyone point me to why we don't support remembering the user choice
> except in some app type? that's new to me.

I would like to know this also... specially since I *can* remember the decision for web sites on desktop (I can choose "Always share location" for geolocation for example). Why don't we let the user choose? He can always change his mind and go to settings and revoke the permission, can't he?
It's not a policy decision. It's just that we don't have a good
mechanism for the user to revoke the permissions. This is a
sufficiently dangerous permission that it's important that that
be right and not something that users need to hunt for.
(In reply to Eric Rescorla (:ekr) from comment #12)
> It's not a policy decision. It's just that we don't have a good
> mechanism for the user to revoke the permissions. This is a
> sufficiently dangerous permission that it's important that that
> be right and not something that users need to hunt for.

So is this something only for this permission, or has that been decided for all the prompt permissions?
(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #13)
> (In reply to Eric Rescorla (:ekr) from comment #12)
> > It's not a policy decision. It's just that we don't have a good
> > mechanism for the user to revoke the permissions. This is a
> > sufficiently dangerous permission that it's important that that
> > be right and not something that users need to hunt for.
> 
> So is this something only for this permission, or has that been decided for
> all the prompt permissions?

I am not taking a position on other permissions. The WebRTC team decided
that *this* permission should not be persistent for Web content until
we had a better mechanism/UX for the users to revoke.
Component: General → WebRTC
Product: Firefox OS → Core
Version: unspecified → Trunk
feedback was still "-" so Ivan following up for why.
Talked to S.C. and he will work on the patch since we are going to add "remember my choice" for gUM permission in 2.0
Assignee: nobody → schien
Whiteboard: [p=1][webRTC]
Target Milestone: --- → 2.0 S2 (23may)
Whiteboard: [p=1][webRTC] → [p=2][webRTC]
I am working on some tests regarding Loop and the interaction in Gaia level, and I'am testing how to 'remember' the choice when asking for video & audio after a gUM request. Currently we are not showing this in Gaia due to [1], but we can test it easily changing [1] with [2], and letting the user to remember the permission _only_ in the case of being an App (if gUM is requested by a website, the 'remember' option is not shown).

With this change applied, I've just realized that I can remember the 'audio' permission, but *not* the 'video'. I'm retrieving 'PERMISSION_DENIED' every time I'm requesting video in gUM after remembering option, and It should not. Probably the scope of this bug should be ensuring that at Gecko level we can remember audio & video as an option, regardless if this is exposed or not in the UI (which is the case in Gaia), and which is supposed to be solved in [3].

Could we try to ensure that this 'remember' is working at Gecko level, and separate the UI in other bug? Wdyt? Thanks!

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/permission_manager.js#L185
[2] https://github.com/borjasalguero/gaia/blob/loop_cs/apps/system/js/permission_manager.js#L199
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=938467
Flags: needinfo?(itsay)
(In reply to Borja Salguero [:borjasalguero] from comment #17)
> 
> Could we try to ensure that this 'remember' is working at Gecko level, and
> separate the UI in other bug? Wdyt? Thanks!

Hi Borja,

The gUM "remember my choice" for video is still under implementation.

This bug was reopened for this purpose and the gecko patch in this bug still needs to be modified for the "remember" of gUM video. S.C. will be working on this gecko patch.

For the UI part, we do have bug 938467 and bug 945111 for it. All three bugs have to be finished to complete the function of gUM "remember my choice"
Flags: needinfo?(itsay)
summary of changes:
1. no prompt for video-capture if permission denied by default or permission granted with no more than one camera available

todos:
1. carry information about permission granted or not in mozChromeEvent
2. handling audio+video request while one of the permission is denied by default
Attachment #8384517 - Attachment is obsolete: true
@schien I got feedback from UX that they like to keep the correct camera selected when the device selector prompt. which means the expect behavior is:

1. per app gUM dialog prompt, select a camera from list (select 'front'), check the 'remember' checkbox, click 'share'.
2. close app
3. open app, camera selection dialog prompt. the list keep the selected camera. ('front' selected)

So gecko may need to store which camera is selected as well.
summary of changes:
1. allow/cancel the permission request if no prompt or user selection is required.
2. introduce |isGranted| to identify the permission prompt is only for user selection or not.
3. cancel the entire request if some permission is denied by default. (require UX to define the behavior for handling partial denied)
Attachment #8421662 - Attachment is obsolete: true
Attachment #8422369 - Flags: feedback?(fabrice)
Comment on attachment 8422369 [details] [diff] [review]
support permission granted/deny for gUM permission, v1

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

::: b2g/components/ContentPermissionPrompt.js
@@ +62,5 @@
> + * @return the default choices for permissions with options, return
> + *         undefined if no option in all requested permissions.
> + */
> +function buildDefaultChoices(aTypesInfo) {
> +  let choices = undefined;

nit: your don't need the |= undefined|
Attachment #8422369 - Flags: feedback?(fabrice) → feedback+
This feature is needed for gUM persistent permission in Firefox OS 2.0.
feature-b2g: --- → 2.0
Note: confirmed with UX, gUM request for video + audio will be denied if one of video/audio permissions is configured to "Denied" in settings app.
(In reply to Shih-Chiang Chien [:schien] (UTC+8) from comment #25)
> Note: confirmed with UX, gUM request for video + audio will be denied if one
> of video/audio permissions is configured to "Denied" in settings app.

That doesn't sound right to me. I would expect the following:

* Audio = "Denied", Video = "Prompt" - should be a permission prompt for video only
* Video = "Denied", Audio = "Allow" - should be automatically granted permissions for audio only
* Audio = "Denied", Video = "Denied" - should be automatically declined permissions for video & audio
Jason, if the app really needs audio and video, does that makes sense to grant permission for only one of them?

It seems to me that if the app asks for both, we should:
- grant if they are both allowed in the settings app.
- deny if they are both denied in the settings app.
- prompt if any is set to prompt in the settings app.
Comment on attachment 8426071 [details] [diff] [review]
support permission granted/deny for gUM permission, v2

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

Clearing the review flag until there is consensus on the expected behavior.
Attachment #8426071 - Flags: review?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #27)
> Jason, if the app really needs audio and video, does that makes sense to
> grant permission for only one of them?

Yes mainly because:

This supports the pajamas use case that's similar to the problem that bug 1013838 talks about. This is actually what desktop & firefox for android already do today with audio/video perm prompts. A user has the ability to choose to grant access to the mic only in a camera & microphone prompt.

> 
> It seems to me that if the app asks for both, we should:
> - grant if they are both allowed in the settings app.
> - deny if they are both denied in the settings app.
> - prompt if any is set to prompt in the settings app.

Agree with all of this.
(In reply to Jason Smith [:jsmith] from comment #29)
> (In reply to Fabrice Desré [:fabrice] from comment #27)
> > Jason, if the app really needs audio and video, does that makes sense to
> > grant permission for only one of them?
> 
> Yes mainly because:
> 
> This supports the pajamas use case that's similar to the problem that bug
> 1013838 talks about. This is actually what desktop & firefox for android
> already do today with audio/video perm prompts. A user has the ability to
> choose to grant access to the mic only in a camera & microphone prompt.
In the current UI design of gUM permission prompt, user can only approve either both camera & microphone or nothing. I don't expect end user will use app permission setting to enter pajamas mode before opening the chat app or accepting the incoming video call.
(In reply to Shih-Chiang Chien [:schien] (UTC+8) from comment #30)

> In the current UI design of gUM permission prompt, user can only approve
> either both camera & microphone or nothing. I don't expect end user will use
> app permission setting to enter pajamas mode before opening the chat app or
> accepting the incoming video call.

We offer a UI to do so in the settings, so we need to be consistent. Be confident users will find the permission trick...
(In reply to Fabrice Desré [:fabrice] from comment #31)
> (In reply to Shih-Chiang Chien [:schien] (UTC+8) from comment #30)
> 
> > In the current UI design of gUM permission prompt, user can only approve
> > either both camera & microphone or nothing. I don't expect end user will use
> > app permission setting to enter pajamas mode before opening the chat app or
> > accepting the incoming video call.
> 
> We offer a UI to do so in the settings, so we need to be consistent. Be
> confident users will find the permission trick...
This use case will only happened after we allow gUM return partial result on Firefox OS, and it is a hacky UI we expose to end user for pajamas mode.
poke omega (UX) in loop
Flags: needinfo?(ofeng)
An app should do pajamas mode in-app, so we don't need to provide that in a hacky way.
My suggestion is that when an app requests video + audio but one of them is "Deny", the request will be denied without prompt.
Flags: needinfo?(ofeng)
(In reply to Omega Feng [:Omega] from comment #34)
> An app should do pajamas mode in-app, so we don't need to provide that in a
> hacky way.
> My suggestion is that when an app requests video + audio but one of them is
> "Deny", the request will be denied without prompt.

I think this is a bad idea - you are trusting the fact that the app will even give the control to the user, which may not be the case. System-based UI instead puts the user first, as that's within gecko/gaia's control, which allows the user to provide a stop gap of what should and should not be shared. This also breaks parity with desktop and Firefox for android, which seems like the wrong direction to me. Our existing user base has been critical about wanting control of what they can and can't share for existing platforms where gum shipped, so anything that goes against that goal is likely going to cause support complaints.
Hi Jason, people who like to control gUM sharing should config the gUM permission as "Always Ask". In addition, we can provide the same level of user control in desktop/fennec by adding "no video" and "no audio" option in the media selection dialog on Firefox OS. In this way, people can have the System-based pajamas mode without hacking it by using partial deny in settings. Does it make sense to you?
Flags: needinfo?(jsmith)
(In reply to Jason Smith [:jsmith] from comment #35)
> (In reply to Omega Feng [:Omega] from comment #34)
> > An app should do pajamas mode in-app, so we don't need to provide that in a
> > hacky way.
> > My suggestion is that when an app requests video + audio but one of them is
> > "Deny", the request will be denied without prompt.
> 
> I think this is a bad idea - you are trusting the fact that the app will
> even give the control to the user, which may not be the case. System-based
> UI instead puts the user first, as that's within gecko/gaia's control, which
> allows the user to provide a stop gap of what should and should not be
> shared. This also breaks parity with desktop and Firefox for android, which
> seems like the wrong direction to me. Our existing user base has been
> critical about wanting control of what they can and can't share for existing
> platforms where gum shipped, so anything that goes against that goal is
> likely going to cause support complaints.

This actually doesn't match my experience at all: the major complaint I
get from application developers on this topic is that they want to have
more unmediated access to the camera/microphone selection (this is why
we are allowing long-term permissions (bug 938467) and why the WebRTC
WG is discussing allowing camera re-selection w/o permissions dialog.

As SC says, if people want the system rather than the application to
enforce partial grants, they should put the system into "always ask"
mode rather than foisting the entire UI problem onto the system (which
has no real context) and taking away from the application, which has
all the context.

As we are now nearing the 40 comment mark, let's move this discussion off
Bugzilla. Shell, can you please arrange some time for Adam, Jesup and Paul
(and Jason and SC if they want to offer opinions) to meet and resolve this?
Flags: needinfo?(sescalante)
(In reply to Shih-Chiang Chien [:schien] (UTC+8) from comment #36)
> Hi Jason, people who like to control gUM sharing should config the gUM
> permission as "Always Ask". In addition, we can provide the same level of
> user control in desktop/fennec by adding "no video" and "no audio" option in
> the media selection dialog on Firefox OS. In this way, people can have the
> System-based pajamas mode without hacking it by using partial deny in
> settings. Does it make sense to you?

That sounds fine to me.

(In reply to Eric Rescorla (:ekr) from comment #37)
> As we are now nearing the 40 comment mark, let's move this discussion off
> Bugzilla. Shell, can you please arrange some time for Adam, Jesup and Paul
> (and Jason and SC if they want to offer opinions) to meet and resolve this?

Okay - I'll hold off on further comments.
Flags: needinfo?(jsmith)
Adding to my request list so I don't lose track of this.
Flags: needinfo?(adam)
Flags: needinfo?(adam)
Based on the conversation at the meeting that EKR proposed above, we reached the following conclusions:

The UX will remain as designed: an attempt to acquire both the camera and the microphone when one is set to persistently deny will result in a failure.

We may consider additional or different UX in the 2.1 timeframe, based on feedback from implementors and users.

If possible, we should add telemetry to peg the number of times this situation arises (access fails due to only one resource being persistently denied) to inform future decisions.

Clearing Shell's needinfo, as the meeting has now taken place.
Flags: needinfo?(sescalante)
We have no telemetry support enabled on b2g, and no way currently to get telemetry data from gaia. Let's hope we'll get direct feedback.
No longer blocks: b2g-getusermedia
Whiteboard: [p=2][webRTC] → [p=2][webRTC]ft:loop
Comment on attachment 8426071 [details] [diff] [review]
support permission granted/deny for gUM permission, v2

resume the review process since the conclusion is made according to comment #40.
Attachment #8426071 - Flags: review?(fabrice)
Attachment #8426071 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/7cdfb038dd19
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: