Closed Bug 1451798 Opened 3 years ago Closed 3 years ago
59 bytes, text/x-review-board-request
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
Priority: -- → P1
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
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+
(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.   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.  https://developer.android.com/reference/android/hardware/Camera.CameraInfo.html#facing  https://developer.android.com/reference/android/hardware/camera2/CameraCharacteristics.html#LENS_FACING
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/cfe13e874f02 Check the variable actually containing the string. r=jib
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.
You need to log in before you can comment on or make changes to this bug.