981113 - [B2G][Gaia][System] A PERMISSION_DENIED error does not appear when denying access to the microphone

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Lionel Mauritson]

Attachment: logcat

Description:
A PERMISSION_DENIED error does not appear when denying access to the microphone as is suggested in the linked test case. Instead the application continues forward and the page must be refreshed to restore functionality.

Repro Steps:
1) Update a Buri to BuildID: 20140307040203
2) Open the browser and go to http://mozilla.github.io/webrtc-landing/gum_test.html
3) Select Audio
>>A permission prompt should appear asking if the user would like to share the microphone
4) Deny access to the microphone

Actual:
No PERMISSION_DENIED error message displays.

Expected:
When I deny permissions a PERMISSION_DENIED error should be seen

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140307040203
Gaia: 04eb7996543f114133d1367f834a4d88b68c60ac
Gecko: b0e7f63c2138
Version: 30.0a1
Firmware Version: v1.2-device.cfg

Keywords: audio video permission mic

Repro frequency: %100
Link to failed test case: https://moztrap.mozilla.org/manage/case/9966/
See attached: logcat

Comment 1

[Jason Smith [:jsmith]]

Does this occur on 1.3?

Comment 2

[Rachel Pribble [:rpribble]]

This does repro on v1.3. No error notification is shown on device.

Environmental Variables:
Device: buri v1.3 MOZ ril
BuildID: 20140310004001
Gaia: 78c30d7ed6f6e30337d6c05453b867f5e97e42bc
Gecko: 00f249d54bda
Version: 28.0
Firmware Version: v1.2-device.cfg

Comment 3

[Jason Smith [:jsmith]]

What about 1.2?

Comment 4

[Rachel Pribble [:rpribble]]

Does repro on v1.2, as well. No PERMISSION_DENIED notification is shown to user.

Environmental Variables:
Device: buri v1.2 MOZ ril
BuildID: 20140220004002
Gaia: 539a25e1887b902b8b25038c547048e691bd97f6
Gecko: 2ea6a65eea23
Version: 26.0
Firmware Version: v1.2-device.cfg

Comment 5

[Jason Smith [:jsmith]]

Might be a site bug, but I'm not sure. I'll bounce this over to the WebRTC component to see what those guys think.

Comment 6

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

I see a Javascript error happened after push the "don't share" button. I'll submit a corresponding patch soon.
>E/GeckoConsole(  126): [JavaScript Error: "aPerm is not defined" {file: "jar:file:///system/b2g/omni.ja!/components/ContentPermissionPrompt.js" line: 347}]

Comment 7

[Jason Smith [:jsmith]]

Attachment: Test Case

Comment 8

[Jason Smith [:jsmith]]

Comment on attachment 8388946 [details]
Test Case

><script text="text/javascript">
>navigator.mozGetUserMedia({audio: true}, function(stream) {
>	console.log('success');
>}, function(err) {
>	console.log('err: ' + err);
>});
></script>

Comment 9

[Jason Smith [:jsmith]]

Attachment: Test Case

Comment 10

[Jason Smith [:jsmith]]

I'm surprised this went undetected this long.

Anyways, this basically means we never fire an onerror callback when someone declines access to getUserMedia. I've verified with my reduced test case that this onerror callback never fires.

Comment 11

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

Likely the reason is that at least on Desktop, failing to answer the request doesn't fire onerror (though that confuses app writers moderately often).  The reason is that the doorhanger is collapsed, but can still be re-opened and accepted.

The side-effect of this is that apps usually don't key off denial much, so the impact of failing to call onerror is diminished in practice.

In this case (though I'm running an inbound build on Helix) it appears similar (with no option to permanently deny): select Don't Share, then touch the Lock icon and it asks again.  We should try to improve the imagery for future releases.

Given the parity level and lack of dependence by real apps on onerror, I would say do not block on this.

Comment 12

[Jason Smith [:jsmith]]

(In reply to Randell Jesup [:jesup] from comment #11)
> Likely the reason is that at least on Desktop, failing to answer the request
> doesn't fire onerror (though that confuses app writers moderately often). 
> The reason is that the doorhanger is collapsed, but can still be re-opened
> and accepted.

This isn't what the bug is talking about. The bug here is talking about the case of explicitly declining the request, not collapsing the existing request.

> 
> The side-effect of this is that apps usually don't key off denial much, so
> the impact of failing to call onerror is diminished in practice.

This isn't right. Many apps including sample webrtc apps always have keyed off onerror to indicate that the camera or microphone was denied explicitly with a clear error to the user. Not having this is bad for partner content, as this removes the developer's ability to handle errors from a permission request. This is a blocker for any content partner planning to take advantage of gUM.

> 
> In this case (though I'm running an inbound build on Helix) it appears
> similar (with no option to permanently deny): select Don't Share, then touch
> the Lock icon and it asks again.  We should try to improve the imagery for
> future releases.

That doesn't sound like the behavior to my understanding of what permission prompts do on Firefox OS. We don't have a concept of a doorhanger on FxOS - we have have modal prompts that require user decision immediately upon prompting.

> 
> Given the parity level and lack of dependence by real apps on onerror, I
> would say do not block on this.

I think this argument is incorrect and doesn't align with our business requirements for partner apps. We have a basic requirement from partners that require error handling in exist in a WebAPI in basic use cases. The parity argument isn't right, given that this is talking about explicit declining of gUM request, which works correctly on FxDesktop & FxAndroid. Nearly every single WebRTC application out there makes use of onerror to report good errors to a user, so the lack of dependency definitely isn't right. This definitely is a content blocker for partner apps.

Comment 13

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

(In reply to Jason Smith [:jsmith] from comment #12)
> (In reply to Randell Jesup [:jesup] from comment #11)
> > Likely the reason is that at least on Desktop, failing to answer the request
> > doesn't fire onerror (though that confuses app writers moderately often). 
> > The reason is that the doorhanger is collapsed, but can still be re-opened
> > and accepted.
> 
> This isn't what the bug is talking about. The bug here is talking about the
> case of explicitly declining the request, not collapsing the existing
> request.

yes.  And on desktop, if you deny the request (instead of ignoring it), it will fire onerror.

> > The side-effect of this is that apps usually don't key off denial much, so
> > the impact of failing to call onerror is diminished in practice.
> 
> This isn't right. Many apps including sample webrtc apps always have keyed
> off onerror to indicate that the camera or microphone was denied explicitly
> with a clear error to the user. Not having this is bad for partner content,
> as this removes the developer's ability to handle errors from a permission
> request. This is a blocker for any content partner planning to take
> advantage of gUM.

You're right that most apps include code for it; my point was that with FF, users rarely Deny in practice (except for testing); they just ignore the doorhanger and let it collapse.  So in practice, Deny isn't a useful user flow.  And yes, this causes annoyance and occasional confusion to developers and may be worth revisiting during future/ongoing UI reviews, but this is how it is on desktop and what they have to deal with.  (Note Chrome has slightly different actions here.)

> > In this case (though I'm running an inbound build on Helix) it appears
> > similar (with no option to permanently deny): select Don't Share, then touch
> > the Lock icon and it asks again.  We should try to improve the imagery for
> > future releases.
> 
> That doesn't sound like the behavior to my understanding of what permission
> prompts do on Firefox OS. We don't have a concept of a doorhanger on FxOS -
> we have have modal prompts that require user decision immediately upon
> prompting.

I realize; I was indicating to you how it appears to function today with an inbound build on my Helix.  It appears that the icon lets you revisit your answer, and that Don't Share collapses it but does not fail the request.

> > Given the parity level and lack of dependence by real apps on onerror, I
> > would say do not block on this.
> 
> I think this argument is incorrect and doesn't align with our business
> requirements for partner apps. We have a basic requirement from partners
> that require error handling in exist in a WebAPI in basic use cases. The
> parity argument isn't right, given that this is talking about explicit
> declining of gUM request, which works correctly on FxDesktop & FxAndroid.
> Nearly every single WebRTC application out there makes use of onerror to
> report good errors to a user, so the lack of dependency definitely isn't
> right. This definitely is a content blocker for partner apps.

On Android, the flow is slightly different.  Don't Share does fire onerror there, but tapping outside the doorhanger collapses it, and touching the location bar and then 'back'ing out to the browser re-displays the gUM premissions request.  However, there is no icon to bring it up.  This is close to desktop.

I do see that it matters more for b2g-specific apps, as you're not falling back on the desktop behavior.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Randell Jesup [:jesup] from comment #13)
> 
> > > The side-effect of this is that apps usually don't key off denial much, so
> > > the impact of failing to call onerror is diminished in practice.
> > 
> > This isn't right. Many apps including sample webrtc apps always have keyed
> > off onerror to indicate that the camera or microphone was denied explicitly
> > with a clear error to the user. Not having this is bad for partner content,
> > as this removes the developer's ability to handle errors from a permission
> > request. This is a blocker for any content partner planning to take
> > advantage of gUM.
> 
> You're right that most apps include code for it; my point was that with FF,
> users rarely Deny in practice (except for testing); they just ignore the
> doorhanger and let it collapse.  So in practice, Deny isn't a useful user
> flow.  And yes, this causes annoyance and occasional confusion to developers
> and may be worth revisiting during future/ongoing UI reviews, but this is
> how it is on desktop and what they have to deal with.  (Note Chrome has
> slightly different actions here.)

Although we don't have a concept of ignoring the permission request on Firefox OS. It's either accept or decline. So this situation is a bit different.

> 
> > > In this case (though I'm running an inbound build on Helix) it appears
> > > similar (with no option to permanently deny): select Don't Share, then touch
> > > the Lock icon and it asks again.  We should try to improve the imagery for
> > > future releases.
> > 
> > That doesn't sound like the behavior to my understanding of what permission
> > prompts do on Firefox OS. We don't have a concept of a doorhanger on FxOS -
> > we have have modal prompts that require user decision immediately upon
> > prompting.
> 
> I realize; I was indicating to you how it appears to function today with an
> inbound build on my Helix.  It appears that the icon lets you revisit your
> answer, and that Don't Share collapses it but does not fail the request.

I just looked at this - this is a bug. Looks like clicking the lock icon is reloading the existing page - weird. I'll open a bug for this.

> 
> > > Given the parity level and lack of dependence by real apps on onerror, I
> > > would say do not block on this.
> > 
> > I think this argument is incorrect and doesn't align with our business
> > requirements for partner apps. We have a basic requirement from partners
> > that require error handling in exist in a WebAPI in basic use cases. The
> > parity argument isn't right, given that this is talking about explicit
> > declining of gUM request, which works correctly on FxDesktop & FxAndroid.
> > Nearly every single WebRTC application out there makes use of onerror to
> > report good errors to a user, so the lack of dependency definitely isn't
> > right. This definitely is a content blocker for partner apps.
> 
> On Android, the flow is slightly different.  Don't Share does fire onerror
> there, but tapping outside the doorhanger collapses it, and touching the
> location bar and then 'back'ing out to the browser re-displays the gUM
> premissions request.  However, there is no icon to bring it up.  This is
> close to desktop.
> 
> I do see that it matters more for b2g-specific apps, as you're not falling
> back on the desktop behavior.

My main concern here is that we don't have a concept of ignorance in Firefox OS, so handling explicit requests is actually far more important than other platforms.

Comment 15

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

Attachment: fix-js-error-while-deny.patch

fix the undefined reference error and provide corresponding test case.

Comment 16

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

try result:
https://tbpl.mozilla.org/?tree=Try&rev=eddac8fd0127

Comment 17

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

I built Helix with this patch and it correctly calls onerror now - I see Permission Denied on the gum_test page, for example, and the testcase works.  (Note: the lock icon still reloads the page; that's a different bug).  Thanks sc!

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/aa706477360a

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/aa706477360a

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Please request approval-mozilla-b2g28 on these patches when you feel it is ready for uplift to 1.3.

Comment 21

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

The current patch doesn't applied to 1.3 because that line of code doesn't exist on that branch. Need further check on why 1.3 is also broken.

Comment 22

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

Doesn't reproduce on my Peak with 1.3 build, I doubt this bug needs to uplift to 1.3. @jsmith, can you double check if 1.3 is affected?

Comment 23

[Jason Smith [:jsmith]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) from comment #22)
> Doesn't reproduce on my Peak with 1.3 build, I doubt this bug needs to
> uplift to 1.3. @jsmith, can you double check if 1.3 is affected?

Confirmed - I'm seeing this reproduce on 1.4, but not 1.3.