Bug: "Bundle CSS" option produces invalid markup when link attributes contain quotes

Example site:

This takes as input some Hugo-generated markup like this:

  <link rel="stylesheet" href="./css/prism.css" media="none" onload="this.media='all';">

when “Bundle CSS” is enabled, this is transformed into this invalid markup:

  <link rel='stylesheet' media='none' onload='this.media=\'all\';' href='https://d33wubrfki0l68.cloudfront.net/css/d14d2a76ca5e85d017fd6f61a99cf4ff925fa2b6/css/prism.css'/>

— it uses backslash-escaped single quotes, which is not how HTML works.

This causes Firefox to complain “Uncaught SyntaxError: invalid escape sequence inspiring-franklin-01af8a.netlify.app:1:11”, and Chrome to complain “Uncaught SyntaxError: Invalid or unexpected token”.

The W3C validator starts complaining with this kind of output:

Error: No space between attributes.

At line 49, column 60

='this.media=\'all\';' href='h

Error: Quote ' in attribute name. Probable cause: Matching quote missing somewhere earlier.

At line 49, column 64

is.media=\'all\';' href='https

I have not seen using the onload attribute as a common practice for CSS files. Could you detail the use case?

In this particular case, the Hugo theme is setting the media property:

in a stylesheet load event:

specified in the “inline event handler” form:

It’s doing this to force Prism (a syntax highlighting engine) to apply to all kinds of media after it is loaded, but that’s beside the point. This is not the way I would do it if I were writing it, but it’s valid. This approach is sometimes taken to log errors, e.g.,

<link … onerror="console.error('Failed to load stylesheet')">

which I expect will be outputted as malformed HTML just as onload is in my original bug report.

I suspect that whatever code you have that’s processing link elements will mess up any attributes containing single quotes — for example,

<link rel="stylesheet" href="css/foo.css" mycustomattr="'iamquoted'">

is valid HTML but will likely break in the same way. Worth writing a test for this…

Well, this bug was already filed in the past:

I guess that’s good, if that’s the right place to report the bug… or bad if it hasn’t been fixed for nearly two years!

1 Like

Yeah, this wasn’t taken up as a priority, especially since there was a workaround mentioned in the issue.

1 Like

I think my main concern is that the workaround is not all that convenient for users of themes and libraries — in my case, for example, that code is pulled in from a Hugo theme in a git submodule somewhere; it’s not code I wrote. Fixing a Hugo theme means forking either the file (which can then diverge as the theme changes) or the whole theme, or sending a PR and hoping the theme developer takes it, and that’s a whole new level of skill for many Hugo users.

I’m just going to leave CSS bundling turned off until this is fixed, because I don’t want the burden of maintaining a forked theme with Netlify workarounds.

This is also a time sink for customers — the error message took me nowhere useful, so I ended up root-causing it myself and filing this.

This is really constructive feedback, @rnewman. We totally hear how this isn’t an optimal solution for now. Thanks for much for taking the time to share. We will follow up on the issue above when changes take place.

1 Like

I have this same issue and am wondering if anyone has any thoughts on which of the two is better, since we’re currently only afforded one. That is, should I bundle CSS or should I use the Inline Critical CSS plugin?

Hmm, maybe I can run both through Insights real quick and see which scores higher and report back…

Honestly, it totally depends on the use case and type of project. Some might prefer to keep CSS in a single file and some might want to fill their HTML with all the CSS. You need to choose one for your needs.