overly aggressive legacy-syntax-warning in Bug 1063808 trips on absent createOffer options arg

RESOLVED FIXED in Firefox 33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jib, Assigned: jib)

Tracking

32 Branch
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox32 unaffected, firefox33 fixed, firefox34 fixed, firefox35 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Poor testing on my part, and no automated tests possible on console warnings. :-(

The warning would erroneously trip when you called:

- createOffer(success,failure) with no options arg, or
- createOffer(success,failure, x) where x = null, {}, undefined or containing unrecognized dictionary members

I've verified locally that this patch fixes it.
Attachment #8492312 - Flags: review?(adam)
Attachment #8492312 - Flags: review?(adam) → review?(rjesup)
Comment on attachment 8492312 [details] [diff] [review]
fixed createOffer options arg legacy-syntax-warning to not trip on absent arg

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

::: dom/media/PeerConnection.js
@@ +556,5 @@
>  
>    createOffer: function(onSuccess, onError, options) {
>  
>      // TODO: Remove old constraint-like RTCOptions support soon (Bug 1064223).
> +    // Note that webidl bindings make o.mandatory implicit while o.optional not.

English - "is not"?
Attachment #8492312 - Flags: review?(rjesup) → review+
Fixed English hopefully. Carrying forward r=jesup.
Attachment #8492312 - Attachment is obsolete: true
Attachment #8492329 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c885916823da
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment on attachment 8492329 [details] [diff] [review]
fixed createOffer options arg legacy-syntax-warning to not trip on absent arg (2) r=jesup

Approval Request Comment
[Feature/regressing bug #]: Bug 1063808
[User impact if declined]: False and misleading warnings in web console on the most basic use of peerConnection.createOffer without an options arg (regression)
[Describe test coverage new/current, TBPL]:
Verified locally. Landed on m-i and m-c.
Unfortunately I know of no easy way to confirm web console output in automated tests.
[Risks and why]: Very low risk. Just changes the if-logic that determines showing off warning.
[String/UUID change made/needed]: none
Attachment #8492329 - Flags: approval-mozilla-beta?
Attachment #8492329 - Flags: approval-mozilla-aurora?
Attachment #8492329 - Flags: approval-mozilla-beta?
Attachment #8492329 - Flags: approval-mozilla-beta+
Attachment #8492329 - Flags: approval-mozilla-aurora?
Attachment #8492329 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.