Closed
Bug 1313966
Opened 8 years ago
Closed 8 years ago
RTCSessionDescription interface doesn't match spec
Categories
(Core :: WebRTC, defect, P1)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: jib)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
The spec has: dictionary RTCSessionDescriptionInit { required RTCSdpType type; DOMString sdp; }; interface RTCSessionDescription { readonly attribute RTCSdpType type; readonly attribute DOMString sdp; (though it doesn't define what the "sdp" member is supposed to be set to when the dictionary doesn't define that member; that needs a spec issue). Our IDL has: dictionary RTCSessionDescriptionInit { RTCSdpType? type = null; DOMString? sdp = ""; }; interface RTCSessionDescription { attribute RTCSdpType? type; attribute DOMString? sdp; which has things writable and nullable when the spec has them as neither.
Flags: needinfo?(jib)
Assignee | ||
Comment 1•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #0) > (though it doesn't define what the "sdp" member is supposed to be set to > when the dictionary doesn't define that member; that needs a spec issue). The spec says [1] of sdp: "if type is rollback, this member can be left undefined." It's perhaps not super-clear, but setting a description with a type other than "rollback" without sdp will fail with InvalidModificationError (because we no longer allow user-modification, so the input must always be output from createOffer() or createAnswer() which never produce a description without sdp). [2][3] Except, we presumably still allow setting a description with type "rollback", but from the same processing steps, that nulls the entire RTCSessionDescription attribute itself instead. [1] http://w3c.github.io/webrtc-pc/#dom-rtcsessiondescriptioninit-sdp [2] http://w3c.github.io/webrtc-pc/#set-description [3] https://github.com/w3c/webrtc-pc/issues/420#issuecomment-162167942
Flags: needinfo?(jib)
Reporter | ||
Comment 2•8 years ago
|
||
> "if type is rollback, this member can be left undefined." Right, for the dictionary. But what does that mean for the resulting RTCSessionDescription object? > but setting a description with a type other than "rollback" without sdp will fail with InvalidModificationError Why exactly? I don't see any evidence for this in the spec. Not least because the spec doesn't actually define any processing model for the constructor; as I said, that needs a spec issue. > but from the same processing steps What processing steps? To be clear, I am looking at this: var desc = new RTCSessionDescription({ type: "answer" }); What is desc.sdp after this call and why?
Flags: needinfo?(jib)
Assignee | ||
Comment 3•8 years ago
|
||
Right, I thought we had deprecated the constructor, my bad. I've opened https://github.com/w3c/webrtc-pc/issues/897
Flags: needinfo?(jib)
Reporter | ||
Comment 4•8 years ago
|
||
Oh, it's _deprecated_. But "deprecated" shouldn't mean "yeah, define that it has to be, but not what it does". ;)
Assignee | ||
Updated•8 years ago
|
Rank: 25
Priority: -- → P2
Assignee | ||
Updated•8 years ago
|
Rank: 25 → 15
Priority: P2 → P1
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•8 years ago
|
||
There are some outstanding issues with making this readonly, so how this for now? [1] https://github.com/w3c/webrtc-pc/issues/573#issuecomment-259103219 [2] https://github.com/w3c/webrtc-pc/issues/897
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jib
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8808848 [details] Bug 1313966 - Add deprecation warnings to writable RTCSessionDescription. https://reviewboard.mozilla.org/r/91580/#review91444
Attachment #8808848 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8808848 -
Flags: review?(drno)
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8808848 [details] Bug 1313966 - Add deprecation warnings to writable RTCSessionDescription. https://reviewboard.mozilla.org/r/91580/#review91490 LGTM. Before we make them readonly we also check if none of our tests actually rely on this being writable. ::: dom/media/PeerConnection.js:232 (Diff revision 1) > + > + If these don't serve a purpose I would prefer to remove them.
Attachment #8808848 -
Flags: review?(drno) → review+
Comment hidden (mozreview-request) |
Comment 10•8 years ago
|
||
Pushed by jbruaroey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1ef70e4bf1a7 Add deprecation warnings to writable RTCSessionDescription. r=drno,smaug
Assignee | ||
Updated•8 years ago
|
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1ef70e4bf1a7
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•