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

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: fixed createOffer options arg legacy-syntax-warning to not trip on absent arg

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.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

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"?

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: fixed createOffer options arg legacy-syntax-warning to not trip on absent arg (2) r=jesup

Fixed English hopefully. Carrying forward r=jesup.

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c885916823da

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c885916823da

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/340677d8a19d
https://hg.mozilla.org/releases/mozilla-beta/rev/02eaea5dce76