Closed
Bug 1451798
Opened 7 years ago
Closed 7 years ago
Video facingMode regression
Categories
(Core :: WebRTC: Audio/Video, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | + | verified |
firefox61 | + | verified |
People
(Reporter: pehrsons, Assigned: pehrsons)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
jib
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details |
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•7 years ago
|
Rank: 9
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
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•7 years ago
|
||
(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
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cfe13e874f02
Check the variable actually containing the string. r=jib
Comment 5•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Comment 6•7 years ago
|
||
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 7•7 years ago
|
||
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+
Comment 8•7 years ago
|
||
bugherder uplift |
Comment 9•7 years ago
|
||
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.
Assignee | ||
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•