1482250 - Mid "0" is a foot-gun (caused some JS library breakage).

Status: RESOLVED WONTFIX

(Core :: WebRTC: Signaling, defect, P2)

Description

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

Bug 1476600 changed mid from e.g. "sdparta_0" to "0", for decent reasons (Bug 1476600 comment 15). Chrome also uses it AFAIK.

Unfortunately, "0" is falsy in JavaScript, causing compatibility grief with some libraries, since

    if (transceiver.mid) {
      // we have mid!
    }

would do nothing for "0" or null.

Since mid is nullable in the spec, should we save ourselves some grief and avoid "0", e.g. start counting from "1"?

I'm flagging this as a regression since it caused some breakage.

[1] http://w3c.github.io/webrtc-pc/#idl-def-rtcrtptransceiver

Comment 1

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

[Tracking Requested - why for this release]: regression, in the sense that new behavior breaks some JS libraries.

Comment 2

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

Example breakage: https://github.com/versatica/mediasoup/issues/213#issuecomment-411857806

Comment 3

[Byron Campen [:bwc]]

Well, I'd say this behavior in JS is the foot-gun, but making this change is probably a good idea.

Comment 4

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=32c1489f44e85031d16740522315805ebd6a3550

Comment 5

[Jeremy Lainé]

Before changing the behaviour again : doesn't Chrome in "Unified Plan" mode also use "0" for the first mid? If so I'd say we are giving libraries a useful heads up that their code is brittle..

Comment 6

[Philipp Hancke [:fippo]]

I've pinged Chrome folks, lets see if they can turn that into an issue themselves. I much prefer the service here :-)

Comment 7

[Byron Campen [:bwc]]

Attachment: Bug 1482250: Start mid strings at 1 instead of 0 because of JS bool conversions. r?jib

Comment 8

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

Comment on attachment 8999186 [details]
Bug 1482250: Start mid strings at 1 instead of 0 because of JS bool conversions. r?jib

Jan-Ivar Bruaroey [:jib] (needinfo? me) has approved the revision.

Comment 9

[Nils Ohlmeier [:drno]]

First of all it would be pretty sad to have JavaScript rule the content of network protocols.

I'm certainly not a JavaScript expert, but according to my limited testing:

  var foo = "1"

is True in JavaScript and

  var foo = "0"

is also True.

So I don't think we should ship this.

Comment 10

[Nils Ohlmeier [:drno]]

Relevant explanation https://stackoverflow.com/questions/7615214/in-javascript-why-is-0-equal-to-false-but-when-tested-by-if-it-is-not-fals

So to me this was a bug in on JavaScript SDP library, and it is fixed on their end already.

Comment 11

[David Durst [:ddurst]]

So do we land this as it is or close it out?

Comment 12

[Nils Ohlmeier [:drno]]

I think we should close it and not land it.

But I would be interested if Byron or jib have different thoughts.

Comment 13

[Byron Campen [:bwc]]

I can see an argument for either way, and have no strong opinion.

Comment 14

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

(In reply to Nils Ohlmeier [:drno] from comment #9)
>   var foo = "0"
> 
> is also True.

Ah, right. As your SO link points out, it's only when comparing against numbers it goes pear shaped, e.g.

    "0" == false

is true. So presumably then the JS library in question was storing mids as numbers? Iñaki is that right?

Unless we have reason to believe this is common, I agree we can drop this fix.

Comment 15

[Philipp Hancke [:fippo]]

https://github.com/w3c/ortc/pull/864 -- note that ORTC is moving away from numeric rids in examples.

Comment 16

[Iñaki Baz Castillo]

Hi guys, the problem here is not so much JavaScript itself and its lazy `==` comparisons, but the usage of the JavaScript sdptransform library:

https://github.com/clux/sdp-transform

Unfortunately, when that library parses a SDP string, every field that could be expressed as number is converted from string to number. This is, if `a=mid:0`, initially the parser will get "0" string, but will later convert it into 0 (number).

I felt the pain in mediasoup: https://github.com/versatica/mediasoup/issues/213

Comment 17

[Nils Ohlmeier [:drno]]

Given that Chrome with RTCPeerConnection({sdpSemantics: 'unified-plan'}) also uses 'a=mid:0' I don't think we should fix it.

Comment 18

[Kohei Yoshino]

Posted a site compatibility note just in case: https://www.fxsitecompat.com/en-CA/docs/2018/rtcrtptransceiver-mid-now-returns-media-id-without-prefix/