Closed Bug 997365 Opened 7 years ago Closed 6 years ago

end support for backwards-compatible facingMode constraint on mobile

Categories

(Core :: WebRTC: Audio/Video, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: jib, Assigned: jib)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 3 obsolete files)

Bug 907352 added code that supports backwards compatible constraints for mobile. The intent was to keep this in for one release to aid transition. This bug is filed as a reminder to remove it.
We can keep this around longer if need be (e.g. for Loop back-end). FYI:

> BACKWARDS-COMPATIBLE OLD FORMAT       | NEW FORMAT
> --------------------------------------+----------------------
> { optional: [{ facingMode:"user" }] } | { facingMode:"user" }
> { mandatory: { facingMode:"user" } }  | { facingMode:"user", require:"facingMode" }
> 

Again, the old format works only on B2G and Android.
Splinter helped me see I forgot to remove a file entirely.
Attachment #8556718 - Attachment is obsolete: true
Attachment #8556718 - Flags: review?(martin.thomson)
Attachment #8556718 - Flags: review?(bugs)
Attachment #8556720 - Flags: review?(martin.thomson)
Attachment #8556720 - Flags: review?(bugs)
Comment on attachment 8556720 [details] [diff] [review]
end support for hold-out mandatory/optional: facingMode constraint on mobile (2)

Review of attachment 8556720 [details] [diff] [review]:
-----------------------------------------------------------------

You need to start using reviewboard.  The UX sucks (apart from the bit where you push to create a review, that's nice), but it's still better than splinter...  In this case, I think that I'm going to trust you that the transplant of the common tests from constraints.js was correct.

::: dom/media/tests/mochitest/test_getUserMedia_constraints.html
@@ -25,4 @@
>  */
> -var desktop_tests = [
> -  { message: "legacy facingMode ignored (desktop)",
> -    constraints: { video: { mandatory: { facingMode:'left' } }, fake: true },

This is still probably a useful test to retain.  Even if 'mandatory' is just another 'somethingunknown', it still holds special meaning by virtue of having been something we respected at one time.
Attachment #8556720 - Flags: review?(martin.thomson) → review+
(In reply to Martin Thomson [:mt] from comment #4)
> You need to start using reviewboard.  The UX sucks (apart from the bit where
> you push to create a review, that's nice), but it's still better than
> splinter...  In this case, I think that I'm going to trust you that the
> transplant of the common tests from constraints.js was correct.

Trust and verify. :-) Here's a diff against constraints.js showing tests unharmed. Still comfy on the fence about reviewboard. It seems to be getting better though.

> This is still probably a useful test to retain.  Even if 'mandatory' is just
> another 'somethingunknown', it still holds special meaning by virtue of
> having been something we respected at one time.

I almost left it in there, so, sure.
Note, I'm going to bitrot your patch when I disable the _mobile test to address bug 1063290.
Attachment #8556720 - Flags: review?(bugs) → review+
Incorporate feedback. Carrying forward r=smaug, mt.
Attachment #8556720 - Attachment is obsolete: true
Attachment #8557178 - Attachment is obsolete: true
Attachment #8557937 - Flags: review+
jib: can you post a try link here thanks!
Flags: needinfo?(jib)
Keywords: checkin-needed
Flags: needinfo?(jib)
https://hg.mozilla.org/mozilla-central/rev/ae5b86a2529d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Docs need to reflect this change in the appropriate places.
Keywords: dev-doc-needed
Note that this bug is over a year old, and the information is outdated. As of 38, Firefox follows the spec and no longer uses the require keyword mentioned in comment 1.

BACKWARDS-COMPATIBLE OLD FORMAT        | SPEC (38+)
---------------------------------------+----------------------
{ optional: [{ facingMode: "user" }] } | { facingMode: "user" }
                                       | { facingMode: { ideal: "user" } }
{ mandatory: { facingMode: "user" } }  | { facingMode: { exact: "user" } }

See also http://stackoverflow.com/questions/28282385/webrtc-firefox-constraints/28911694#28911694
Blocks: 1050930
You need to log in before you can comment on or make changes to this bug.