Update structured cloning for recent changes to HTML #1810
Conversation
|
This seems unlikely to be correct, as it references a specification that has known bugs in the area of serialization and cloning. |
|
Aha, I assumed the spec was decent given it is a Recommendation. [Update] Is html 5.3 any better? |
|
No. The root problem is that the editors of that specification seem to copy our work (in the HTML Standard) blindly and incompletely, which leads to lots of bugs. |
|
Could you say more about what the bugs are so we can decide if we can use this in WebRTC. |
|
I'm sorry, I'm not in the business of helping specs add buggy dependencies that browsers cannot feasibly ship. I would suggest not using this in Web RTC, and instead using my original pull request, which depends on the actual maintained definition of structured cloning that is implemented in browsers. |
|
Updated, now identical to @domenic's original PR. Perhaps we should use that one instead and close this PR. |
|
As for why it matters, the HTML Living Standard is the one we consider "authoritative" in Blink, i.e. we try to match it or fix it when it's wrong. It's also what people working in web-platform-tests will need to look at, since it's the most upstream and well maintained. |
|
@domenic can you take a look now? |
webrtc.html
Outdated
|
|
||
| <li>Set <var>serialized</var>.[[\certificate]] to | ||
| a copy of the unstructured binary data in | ||
| <var>value</var>.[[<a>certificate</a>]].</li> |
There was a problem hiding this comment.
It's [[Certificate]] with upper-case 'C' right? Also, we probably want <var>value</var>.[[\Certificate]]. Otherwise I see squiggly red underlines in the spec.
webrtc.html
Outdated
| a copy of the unstructured binary data in | ||
| <var>value</var>.[[<a>certificate</a>]].</li> | ||
|
|
||
| <li>Set <var>serialized</var>.[[\handle]] be a serialization of the |
|
I need to look into your comments @jan-ivar |
|
|
||
| <ol> | ||
| <li>Initialize <var>value</var>'s <code>expires</code> attribute to | ||
| contain <var>serialized</var>.[[\Expires]].</li> |
There was a problem hiding this comment.
nit: Should we link these slots as well?
There was a problem hiding this comment.
I don't think that is necessary - the definition is like 10 lines before. WDYT?
|
I think this one is ready. However we should probably hold off merging until we've issued a new CR (if we do that anytime soon). |
Fixes #1089 (eventually, perhaps).
Do not merge - not ready (respec errors among other things).
@domenic PTAL.
Preview | Diff