Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to serialize CDATASection nodes #2

Closed
TechQuery opened this issue Nov 15, 2018 · 6 comments
Closed

Failed to serialize CDATASection nodes #2

TechQuery opened this issue Nov 15, 2018 · 6 comments

Comments

@TechQuery
Copy link
Contributor

TechQuery commented Nov 15, 2018

Actual

const { JSDOM } = require('jsdom');

const {window: {document, XMLSerializer}} = new JSDOM();

const element = document.createElement('style'),
    documentXML = document.implementation.createDocument(null, 'xml');

element.append( documentXML.createCDATASection('a > b { }') );

console.log( (new XMLSerializer()).serializeToString( element ) );

//  InvalidStateError: Failed to serialize XML: Only Nodes and Attr objects can be serialized

Expected

https://codepen.io/tech_query/pen/KrmxmQ

Environment

{ npm: '6.4.1',
  ares: '1.10.1-DEV',
  cldr: '32.0',
  http_parser: '2.8.0',
  icu: '60.1',
  modules: '57',
  napi: '3',
  nghttp2: '1.32.0',
  node: '8.12.0',
  openssl: '1.0.2p',
  tz: '2017c',
  unicode: '10.0',
  uv: '1.19.2',
  v8: '6.2.414.66',
  zlib: '1.2.11' }

Windows 10.0.17134.407

@domenic
Copy link
Member

domenic commented Nov 15, 2018

When I try to run this code in Node.js, I get a syntax error about import not being supported. Can you edit it to be valid JavaScript code that we can run and reproduce the issue?

You also deleted other important parts of the issue template, like version numbers.

We'll reopen when you've done so and confirmed that the problem still occurs.

@domenic domenic closed this as completed Nov 15, 2018
@TechQuery
Copy link
Contributor Author

TechQuery commented Nov 16, 2018

@domenic There's no Issue template in this repository...

In my opinion, there's no relationship with ECMAScript syntax or Runtime environment about this bug. I located the code which throw this error with the Call stack's help, XMLSerializer didn't take CDATASection into account...

I think we should follow the DOM specification~

@domenic
Copy link
Member

domenic commented Nov 19, 2018

Sorry about that, I thought we were in the jsdom repository. Still, it's good practice to produce code that the people you're reporting the issue to can actually run.

@domenic domenic reopened this Nov 19, 2018
@TechQuery
Copy link
Contributor Author

@domenic How about my PR? Is there anything need to change?

@Zirro
Copy link
Member

Zirro commented Dec 3, 2018

So... the DOM Parsing and Serialization specification which defines XMLSerializer does not include instructions for serializing a CDATASection node, and the main DOM specification does not supersede it by itself. While we usually follow the specifications closely, current browsers seem to serialize CDATASection nodes without issue so making an exception here may be appropriate.

@domenic, do you know if this is a case where the DOM Parsing and Serialization specification has not kept up with the main DOM specification?

@domenic
Copy link
Member

domenic commented Dec 11, 2018

I think this is indeed a part where DOM P&S has not kept up. @Zirro has filed w3c/DOM-Parsing#38 for us. @TechQuery, can you add a link to w3c/DOM-Parsing#38 in your pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants