Video facingMode regression

VERIFIED FIXED in Firefox 60

Status

()

defect
P1
normal
Rank:
9
VERIFIED FIXED
Last year
Last year

People

(Reporter: pehrsons, Assigned: pehrsons)

Tracking

60 Branch
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 unaffected, firefox60+ verified, firefox61+ verified)

Details

Attachments

(1 attachment)

Calling gUM with a facingMode constraint has regressed in 60. Bug 1299515 is the likely suspect so marking it for now.

STR
1 Use an android device with two cameras
2 Load https://jsfiddle.net/jib1/03ng8wva/
3 Click Start!

Expected:
Permission prompt shows "Front facing camera" first in the list, and selected by default

Actual:
Permission prompt shows "Back facing camera" first in the list, and selected by default
Assignee

Updated

Last year
Rank: 9
Priority: -- → P1
Assignee

Updated

Last year
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 2

Last year
mozreview-review
Comment on attachment 8965667 [details]
Bug 1451798 - Check the variable actually containing the string.

https://reviewboard.mozilla.org/r/234524/#review240384

Wow, what a footgun Move() is here. Is there really no better pattern for this?
Attachment #8965667 - Flags: review?(jib) → review+
Assignee

Comment 3

Last year
(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #2)
> Comment on attachment 8965667 [details]
> Bug 1451798 - Check the variable actually containing the string.
> 
> https://reviewboard.mozilla.org/r/234524/#review240384
> 
> Wow, what a footgun Move() is here. Is there really no better pattern for
> this?

The proper way IMO would be to do these checks in the platform backend and signal them up to the MediaEngineSource through IPC. At least on Android this is provided by the system APIs. [1] [2]

A simpler fix to remove the footgun here could be to lift this logic out to a local function that takes a string (the device name) and returns a `Maybe<VideoFacingModeEnum>`.

Feel free to file followups for either of these approaches.


[1] https://developer.android.com/reference/android/hardware/Camera.CameraInfo.html#facing
[2] https://developer.android.com/reference/android/hardware/camera2/CameraCharacteristics.html#LENS_FACING

Comment 4

Last year
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cfe13e874f02
Check the variable actually containing the string. r=jib

Comment 5

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/cfe13e874f02
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee

Comment 6

Last year
Comment on attachment 8965667 [details]
Bug 1451798 - Check the variable actually containing the string.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1299515
[User impact if declined]: Websites using camera capture aimed at mobile (and some other platforms potentially) might end up showing the wrong camera.
[Is this code covered by automated tests?]: No, we don't have devices with real cameras in automation.
[Has the fix been verified in Nightly?]: No. But I've verified locally.
[Needs manual test from QE? If yes, steps to reproduce]: Yes. See comment 0.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Trivial change
[String changes made/needed]: None
Attachment #8965667 - Flags: approval-mozilla-beta?
Comment on attachment 8965667 [details]
Bug 1451798 - Check the variable actually containing the string.

Fix a new regression in Fx60. Approved for 60.0b12.
Attachment #8965667 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Device:
 - Samsung Gala

Verified in the latest Nightly (2017-04-13) and Beta (60.0b12) builds following the steps provided in Comment 0. The issue is no longer reproducible, marking this as verified.
Status: RESOLVED → VERIFIED
Assignee

Updated

Last year
See Also: → 1469319
You need to log in before you can comment on or make changes to this bug.