url([...]url([...])[...]) causes styles wrapping to fail completely (although valid CSS)

[strarsis]

Describe the bug
Some inline SVG strings in CSS cause the Gutenberg editor to fail the wrapping of the theme styles.
Example SVG (reduced to a valid minimum (but useless) SVG that reproduces the issue in Gutenberg):

<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'><defs><style>.b{clip-path:url(#a);}</style></defs></svg>

Many CSS postprocessors (SASS/PostCSS) inline small SVGs for optimization using data:image:svg+xml;charset=utf-8,[escaped SVG].

To reproduce
Steps to reproduce the behavior:

1. Use a theme that contains an affected inline SVG string in its styles, e.g.

body h2 {
  color: white !important;
}

.wp-block-group {
  color: blue !important;
  background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E%3Cdefs%3E%3Cstyle%3E.b%7Bclip-path:url(%23a);%7D%3C/style%3E%3C/defs%3E%3C/svg%3E");
  color: red !important;
}

body h2 {
  color: grey !important;
}

2. Open a post/page on which the aforementioned styles are applied in the Gutenberg editor.
3. Ensure a core/group and core/heading (H2!) block with some example text being present in the post/page.
4. Notice that the red color is not applied, unless the inlined SVG is removed.
   The blue color isn't applied either - the whole declaration that selects .wp-block-group is not applied. But even the styles for the heading below aren't applied, the block got neither white or grey color, hence the whole stylesheet isn't applied when this bug hits.
   Error in console:

Error while traversing the CSS: Error: undefined:2:27: missing '}'

Expected behavior
All styles are applied to editor styles wrapper correctly, as usual.

Additional context
WordPress 5.4
Gutenberg 7.8.1

The issue is caused by the succession of an url(...) reference (no # needed for this bug btw.) and then an "unnecessary" semicolon (when it is the last CSS property in declaration).

Further reduced test case with invalid but bug-reproducing SVG:

.wp-block-group {
  background-image: url("data:image/svg+xml,%3Csvg%3E.b%7Bclip-path:url(test);%7D%3C/svg%3E");
  color: red !important;
}

Minimal version that causes the bug:

.wp-block-group {
  background: url("url(t);");
}

The parser doesn't like url() in urlI) although this CSS validates with https://jigsaw.w3.org/css-validator/validator.

Gutenberg uses its own CSS parser implementation:
https://github.com/WordPress/gutenberg/blob/bb24bbef1790b165ca8608d4cee7a5ec6e85c234/packages/block-editor/src/utils/transform-styles/ast/parse.js
Wouldn't it be a good idea to use something existing like what PostCSS does (and works with native JavaScript)? There is even a PostCSS plugin for exactly this.

[aduth]

Gutenberg uses its own CSS parser implementation:
https://github.com/WordPress/gutenberg/blob/bb24bbef1790b165ca8608d4cee7a5ec6e85c234/packages/block-editor/src/utils/transform-styles/ast/parse.js
Wouldn't it be a good idea to use something existing like what PostCSS does (and works with native JavaScript)? There is even a PostCSS plugin for exactly this.

My understanding is that it's not Gutenberg's own, but rather adapted from reworkcss/css:

gutenberg/packages/block-editor/src/utils/transform-styles/ast/index.js

Lines 1 to 2 in ce1a4d9

	// Adapted from https://github.com/reworkcss/css
	// because we needed to remove source map support.

I'm not sure if the bug originates there. Seeing that the error is during the traversal, it could be.

I'm also not sure how viable the proposed PostCSS plugin is. Notably, this logic is run at runtime, not during build. Unclear how well PostCSS would behave in this environment.

[strarsis]

@aduth: I agree, using a whole framework that may have issues running in the browser natively because of one (rather simple) plugin for it, wouldn't be a good design choice.

Because Gutenberg uses React, what about using existing mechanisms for React for applying external styles? https://www.sitepoint.com/style-react-components-styled-components/
It may be even possible to add proper hot reloading of styles and make it possible to development in Gutenberg with BrowserSync.

Using a CSS parser:
A native CSS parser API is planned (for Houdini project).
There seems to be some kind of polyfill/implementation for this https://github.com/tabatkins/parse-css.
I had very good experience using CSSTree for implementing CSS support in svgo. It is also designed to run in the browser.

[strarsis]

@aduth: Alright, I reimplemented this using CSSTree, the necessary code is very short and the whole thing works nicely in the browser. You can try a working implementation in a Codepen: https://codepen.io/strarsis/pen/RwWRqZv

It downloads an external stylesheet (https://gist.githubusercontent.com/strarsis/71143c72942bc8211191fec9d12286bb/raw/3c4cad97b5c0cb7b7192f0e98b7e69b5e9e0397d/test.out.css), remaps html and body to .editor-styles-wrapper and then adds it as styles element to the document.
The code also works correctly with the aforementioned styles that contain url in url. 😸
It also injects an inline sourcemap (inline sourcemaps in <style> work well in Chrome + Firefox and probably the other browsers, too) and the developer can now see the actual stylesheet file and number instead just <style>. It even reuses an existing sourcemap, so the original, unprocessed SASS/PostCSS processed CSS files can be viewed by the developer!

Points that speak for using CSSTree:

• Designed to run fast in browser and nodejs.
• Used in some major projects like CSSO and SVGO.
• Using that library requires only very few and robust code.
• The library already implemented all the parsing and is improving all the time.
• Sourcemap support, even for existing sourcemaps!

That's the whole code - it depends on three packages that are all explicitly supported and working in browser:

function fetchAndWrapStylesheet(cssUrl) {
  return fetch(themeCssUrl)
  .then(function(data) {
    return data.text();
  })
  .then(function(originalCss) {
    let remappedStyles = remapStyles(originalCss, cssUrl);
    let remappedCss    = remappedStyles.css;

    let sourceMap = remappedStyles.map;
    sourceMap.setSourceContent(cssUrl, originalCss); // necessary?

    let originalSourcemap = '';
    let originalSourcemapUriMatch = originalCss.match(/\/\*\# sourceMappingURL=data:application\/json;base64,(.*)\*\//);
    if(originalSourcemapUriMatch.length > 1) {
      let originalSourcemapUri = originalSourcemapUriMatch[1];
      originalSourcemap = window.atob(originalSourcemapUri);
    }

    // no existing sourcemap, only use own remapping sourcemap
    if(originalSourcemap === '') {
      applyCss(remappedCss, sourceMap);
      return;
    }

    // reuse existing sourcemap on top of remapping sourcemap
    return new SourceMapConsumer(originalSourcemap)
    .then(function(originalMapConsumer) {
      sourceMap.applySourceMap(originalMapConsumer, cssUrl);
      originalMapConsumer.destroy(); // clean up

      applyCss(remappedCss, sourceMap);
    });
  });
}

function applyCss(remappedCss, sourceMap) {
  let sourceMapInline = inlineSourceMapComment(sourceMap.toString(), {block: true});
  let remappedCssWithSourcemap = remappedCss + "\n" + sourceMapInline;
  addStylesheetElementWithCss(remappedCssWithSourcemap);
}

function addStylesheetElementWithCss(css) {
  let head  = document.head || document.getElementsByTagName('head')[0],
      style = document.createElement('style');
  head.appendChild(style);
  style.type = 'text/css';
  style.appendChild(document.createTextNode(css));
}

const WRAPPER_CLASSNAME    = 'editor-styles-wrapper';
const WRAPPER_SELECTOR_OBJ = {
        type: 'ClassSelector',
        name: WRAPPER_CLASSNAME,
      };
const WRAPPER_SELECTOR_AST = csstree.fromPlainObject(WRAPPER_SELECTOR_OBJ);
function remapStyles(css, cssUrl) {
  var ast = csstree.parse(css, {
    filename:  cssUrl,
    positions: true,
  });

  csstree.walk(ast, function(node, item, list) {
    if(node.type !== 'TypeSelector') return; // skip
    if(node.name !== 'body' && node.name !== 'html') return; // skip

    let wrapper_selector_item = list.createItem(WRAPPER_SELECTOR_AST);
    list.replace(item, wrapper_selector_item);
  });

  return csstree.generate(ast, {
    sourceMap: true,
  });
}

// init the mozilla sourcemap library (browser)
let sourceMap = window.sourceMap;
let SourceMapConsumer = sourceMap.SourceMapConsumer;

sourceMap.SourceMapConsumer.initialize({
  'lib/mappings.wasm': 'https://unpkg.com/source-map@0.7.3/lib/mappings.wasm',
});

[aduth]

@strarsis Thanks for diving into this. For full transparency, there might be a short delay before I can revisit this topic.

In the meantime, you may also be interested in the effort proposed at #20797. As I understand it, one of the advantages being proposed with the approach there is that it removes the need for these transformations altogether, since content can scale natively within the frame.

[strarsis]

@aduth: Until the new paradigm using iframes is decided on and implemented, the existing wrapping mechanism could be improved. I prepared a PR #21742. It is my first PR, so there may be still some things to improve before it could be considered for being merged.
The PR doesn't include sourcemap-handling yet (but adds a foundation for it with css-tree parser) because for this the transform-styles method needs to know the URL of the CSS. Also I don't know what baseUrl does, is this meant for URL rewriting?

[strarsis]

New editor styles transformation approach using PostCSS is now merged (many thanks @zaguiini)!