-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
url([...]url([...])[...]) causes styles wrapping to fail completely (although valid CSS) #21569
Comments
My understanding is that it's not Gutenberg's own, but rather adapted from reworkcss/css:
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. |
@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/ Using a CSS parser: |
@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 Points that speak for using CSSTree:
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',
}); |
@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. |
@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. |
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):
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:
core/group
andcore/heading
(H2!) block with some example text being present in the post/page.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:
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:
Minimal version that causes the bug:
The parser doesn't like
url()
inurlI)
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.
The text was updated successfully, but these errors were encountered: