Closed Bug 827145 Opened 7 years ago Closed 7 years ago

Need to return denied from getUserMedia permissions if an error is returned due to no devices

Categories

(Firefox :: Device Permissions, defect)

x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 21
Tracking Status
firefox20 + fixed
firefox21 + fixed

People

(Reporter: jesup, Assigned: Dolske)

References

Details

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

Attachments

(1 file, 1 obsolete file)

See the backend in bug 802656

Hotplug bug will be filed ASAP
Whiteboard: [getUserMedia][blocking-gum+]
Dao: any progress on this one? the backend is in
We're trying to ship this as part of FF 20. So noming for tracking.
(In reply to Jason Smith [:jsmith] from comment #2)
> We're trying to ship this as part of FF 20. So noming for tracking.

What's the user experience without this fix? A lack of a user-facing error message when getUserMedia is used without a device attached?
(In reply to Alex Keybl [:akeybl] from comment #3)
> (In reply to Jason Smith [:jsmith] from comment #2)
> > We're trying to ship this as part of FF 20. So noming for tracking.
> 
> What's the user experience without this fix? A lack of a user-facing error
> message when getUserMedia is used without a device attached?

The developer who was using this API would have no knowledge that the API request failed and wouldn't know that it was due to user denial of permissions. It's a developer-facing API, so the developer experience is impacted for removing the ability to pick this up. The user impact is basically the consequence of the developer not having it - which could result in probably poor error understanding.

We originally marked this as blocking for shipping for FF 20 for gUM v1 because the device integration to audio and video input devices is a critical fundamental piece of the API. Anything that would impact incorrect device integration (whether it would be incorrect on what happened or no error callback) would deteriorate the developer experience significantly for developers consuming this API.

Maire probably could give more of a product perspective here if need be. But that's my take from the QA side.
Jason's analysis is correct.  WebRTC is a developer feature, and developers who are writing webrtc applications need this functionality to do proper error handling.  The backend of this functionality has already landed.  We simply need the front-end UI piece to land.
OK - thanks for the explanation all. We'll track this for release to ensure that our API is hardened and provides the ability to display proper error messages. Do we have an ETA for this change? Would be great to get the fix while FF20 is still on mozilla-aurora (1 more week).
Attached patch Patch v.1 (obsolete) — Splinter Review
How much of a proper error message do you want?

This is a minimal patch, but means the page gets a "PERMISSION_DENIED" error, just as if the user had clicked deny. Better-than-nothing for omguplift, or should we do better right away?

I think the a-bit-more-effort patch is to hook up the ability to either pass in a string (via the current observer's |subject| param), or to add a new observer for getUserMedia:response:nodevices. Preference?
Attachment #714704 - Flags: feedback?(rjesup)
Well, my patch in the backend bug (see comment 0) invokes the onError callback with "NO_DEVICES_FOUND") to the onError fro GetUserMediaDevices, so I've passed that.  Returning PERMISSION_DENIED to the app is ok, but NO_DEVICES_FOUND would be better.
Attached patch Patch v.2Splinter Review
Waiting on my build to finish, but I think this will work. (Famous last words)
Assignee: dao → dolske
Attachment #714704 - Attachment is obsolete: true
Attachment #714704 - Flags: feedback?(rjesup)
Attachment #714729 - Flags: review?(rjesup)
Yes, patch is working...

On my Windows desktop w/o any media clicking a button on http://mozilla.github.com/webrtc-landing/gum_test.html gives me a NO_DEVICES_FOUND error in the page, and on my MBP clicking allow/deny for the camera both work as expected.
Attachment #714729 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/8786f346fc6f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Needs an uplift to Aurora preferably before we go to build for Beta 1.
Keywords: verifyme
QA Contact: jsmith
Whiteboard: [getUserMedia][blocking-gum+] → [getUserMedia][blocking-gum+][webrtc-uplift]
Comment on attachment 714729 [details] [diff] [review]
Patch v.2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Users without a camera or mic won't get an error returned to the application.  This will impact the ability of authors to produce a consistent UI for users.
Testing completed (on m-c, etc.): On m-c.  Locally tested by dolske and myself.
Risk to taking this patch (and alternatives if risky): Minimal risk.
String or UUID changes made by this patch: UNKNOWN_ERROR returned to application if no error message is supplied (no l10n needed, not a user-facing string)

Backend is already in FF20
Attachment #714729 - Flags: approval-mozilla-aurora?
Attachment #714729 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 842442
Verified on 2/18 with one followup - bug 842442. We appear to be failing an edge case when you request audio with only a camera available or video with only a mic available.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Whiteboard: [getUserMedia][blocking-gum+][webrtc-uplift] → [getUserMedia][blocking-gum+]
Component: General → Device Permissions
You need to log in before you can comment on or make changes to this bug.