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

Could not parse CSS Stylesheet, when inline style tag font-family has space #2123

Closed
AlbinoDrought opened this issue Jan 24, 2018 · 1 comment · Fixed by #2132
Closed

Could not parse CSS Stylesheet, when inline style tag font-family has space #2123

AlbinoDrought opened this issue Jan 24, 2018 · 1 comment · Fixed by #2132

Comments

@AlbinoDrought
Copy link

AlbinoDrought commented Jan 24, 2018

At first I was blaming this on CSSOM, but after some digging it seems like the content passed to CSSOM here is not complete. (seems to be how the parser works)

Basic info:

  • Node.js version: v9.2.1
  • jsdom version: 11.6.0

Minimal reproduction case

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

const dom = new JSDOM(`
<html>
  <head></head>
  <body>
    <style>
    .cool-class {
        font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
    }
    </style>
    <p class="cool-class">
    Hello!
    </p>
  </body>
</html>
`);

Output:

Error: Could not parse CSS stylesheet
    at exports.evaluateStylesheet (jsdom/lib/jsdom/living/helpers/stylesheets.js:23:21)
    at HTMLStyleElementImpl._childTextContentChangeSteps (jsdom/lib/jsdom/living/nodes/HTMLStyleElement-impl.js:20:5)
    at TextImpl.replaceData (jsdom/lib/jsdom/living/nodes/CharacterData-impl.js:73:23)
    at TextImpl.set data [as data] (jsdom/lib/jsdom/living/nodes/CharacterData-impl.js:21:10)
    at JSDOMParse5Adapter.insertText (jsdom/lib/jsdom/browser/parse5-adapter-parsing.js:115:22)
    at module.exports.Parser._insertCharacters (parse5/lib/parser/index.js:558:26)
    at Object.insertCharacters (parse5/lib/parser/index.js:1014:7)
    at module.exports.Parser._processToken (parse5/lib/parser/index.js:607:38)
    at module.exports.Parser._processInputToken (parse5/lib/parser/index.js:639:14)
    at module.exports.Parser._runParsingLoop (parse5/lib/parser/index.js:409:14) 
    .cool-class {
        font-family: "Helvetica
Error: Could not parse CSS stylesheet
    at exports.evaluateStylesheet (jsdom/lib/jsdom/living/helpers/stylesheets.js:23:21)
    at HTMLStyleElementImpl._childTextContentChangeSteps (jsdom/lib/jsdom/living/nodes/HTMLStyleElement-impl.js:20:5)
    at TextImpl.replaceData (jsdom/lib/jsdom/living/nodes/CharacterData-impl.js:73:23)
    at TextImpl.set data [as data] (jsdom/lib/jsdom/living/nodes/CharacterData-impl.js:21:10)
    at JSDOMParse5Adapter.insertText (jsdom/lib/jsdom/browser/parse5-adapter-parsing.js:115:22)
    at module.exports.Parser._insertCharacters (parse5/lib/parser/index.js:558:26)
    at Object.insertCharacters (parse5/lib/parser/index.js:1014:7)
    at module.exports.Parser._processToken (parse5/lib/parser/index.js:607:38)
    at module.exports.Parser._processInputToken (parse5/lib/parser/index.js:639:14)
    at module.exports.Parser._runParsingLoop (parse5/lib/parser/index.js:409:14) 
    .cool-class {
        font-family: "Helvetica 

How does similar code behave in browsers?

The HTML works fine in browsers, and sets the font properly: https://jsfiddle.net/tveprk11/


Removing the space from "Helvetica Neue" allows the code to run without errors.

@domenic
Copy link
Member

domenic commented Jan 24, 2018

This probably regressed in 11.6.0 due to our parsing changes. IIRC parse5 feeds us new text nodes if there is a space. We probably need to do something similar to what we do for script elements.

I'll try to fix/hack around soon.

domenic added a commit that referenced this issue Jan 27, 2018
This replaces our previous hacky <script>-only close tag detection, which only worked in non-empty <script> cases, with a more general hacky version that works for all elements, including empty elements, by monkeypatching parse5. (Eventually inikulin/parse5#237 should give us a better solution, but for now we lock our parse5 version and monkeypatch it.)

In particular, this allows us to not parse stylesheets before their close tag is encountered, which fixes #2123 and fixes #2131.
domenic added a commit that referenced this issue Jan 28, 2018
This replaces our previous hacky <script>-only close tag detection, which only worked in non-empty <script> cases, with a more general hacky version that works for all elements, including empty elements, by monkeypatching parse5. (Eventually inikulin/parse5#237 should give us a better solution, but for now we lock our parse5 version and monkeypatch it.)

In particular, this allows us to not parse stylesheets before their close tag is encountered, which fixes #2123 and fixes #2131.
domenic added a commit that referenced this issue Jan 29, 2018
This replaces our previous hacky <script>-only close tag detection, which only worked in non-empty <script> cases, with a more general hacky version that works for all elements, including empty elements, by monkeypatching parse5. (Eventually inikulin/parse5#237 should give us a better solution, but for now we lock our parse5 version and monkeypatch it.)

In particular, this allows us to not parse stylesheets before their close tag is encountered, which fixes #2123 and fixes #2131.
domenic added a commit that referenced this issue Jan 30, 2018
This replaces our previous hacky <script>-only close tag detection, which only worked in non-empty <script> cases, with a more general hacky version that works for all elements, including empty elements, by monkeypatching parse5. (Eventually inikulin/parse5#237 should give us a better solution, but for now we lock our parse5 version and monkeypatch it.)

In particular, this allows us to not parse stylesheets before their close tag is encountered, which fixes #2123 and fixes #2131.
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

Successfully merging a pull request may close this issue.

2 participants