Fix or replace CSS Parser that transforms editor styles

[kraftner]

What problem does this address?

The currently used CSS parser implementation to transform editor styles is fragile and breaks on multiple kinds of perfectly valid CSS.

Since there are currently multiple issues and PRs with varying descriptions that seem to all point back to this very problem I thought I'd summarize and consolidate them here to streamline further steps.

Examples / Other issues

To begin some examples of what we are talking about:

• transformStyles not working correctly #16408
• Error while traversing the CSS: property missing ':' #19930
• url([...]url([...])[...]) causes styles wrapping to fail completely (although valid CSS) #21569

What is your proposed solution?

Earlier attempts

The parsing is currently done here which is an adapted fork of https://github.com/reworkcss/css. It seems though that the upstream package has identified the issue as well but has no fix and isn't actively maintained anyway (the last release being from early 2020)

• Parser error with SVG datauri inlining reworkcss/css#67 (comment)

There has been an attempt to fix it by @oxyc based on the upstream issue:

• Handle closing parenthesis within data url in transform-styles (#16408) #17146

This got closed in favor of another approach by @strarsis that seemed to be better but then got stalled due to an increase in bundle size

• Use css-tree for block-editor style transformations (3). #25514

Finally at the end of this PR @strarsis proposed another approach using "native Browser API and a CSS selector parser" that might be a viable middle ground: #25514 (comment)

Next steps

I currently see 3 possible ways to solve this:

• Try to work with upstream to merge the existing deviations in the fork and fix the underlying issues with the parser.
• Fix the issue in the fork here.
• Further investigate the approach of Use css-tree for block-editor style transformations (3). #25514 to replace the parser with something more solid.

Finally there currently is the endeavor to iframe the editor content so the original CSS can be used making the whole transformation including all these issues go away

• Consider iframing the editor canvas/content #20797

Since this might still take some time though it might still be a good idea to fix this in the meantime.

What would probably be nice in any case and a good first step is to add some tests covering the above issues since there currently is pretty much no coverage at all for this.

[kraftner]

Possible workaround

A big chunk of the issues seem to stem from the fact that nested ) inside a CSS declaration cause parsing to greedily include following declarations. This can often be worked around by adding a line break and or a comment after the offending declaration.

So for example, this line isn't properly parsed.

@font-face{src:url() format(")")}.a[b="c"]{}

but is if you are writing it like this:

@font-face{src:url() format(")")}
/*fix*/
.a[b="c"]{}

[fabiankaegy]

Thanks for raising this issue. For the longest time we've not used the official add_editor_style way of adding the editor styles but instead enqueued then manually on the enqueue_block_editor_assets hook. But with more and more editors using the iframed editor (which I'm a huge fan of because having that everywhere would solve this entirely) we switched away from that to the add_editor_style approach.

The issue we keep running into, is that unlike the browser, the editor style parser used in here does not fail gracefully but instead fails completely when there are any issues in the CSS input.

This would be fine if the parser were perfect and had no issues with perfectly valid syntax. But with these issues it causes problems.

So thanks again for raising this issue and for taking anyone also taking a look at this :)

[Antonio-Laguna]

If this has bitten you because of PostCSS, you might want to know that if you're using postcss-preset-env we've changed the behaviour of our plugins so they don't remove the last semicolon on the last declaration which should prevent at least one scenario for this parser's failure.

If you detect any issue that can be prevented (or is caused by us) I'd be more than happy to take a look.

[patrickwc]

I think this is the place to report issues with the CSS Parser. I noticed that after updating to WP 6.0, during theme development when Gutenberg was loading our unminified main.css stylesheet none of our styles were being applied properly, but it worked fine with the .min.css file. I have isolated the issue. When using an inline svg which has a style attribute, if a semi-colin is included at the end of the attribute, the parser breaks and no styles are loaded.

E.g. this breaks and fails to load any CSS:

		&:after {
			margin-top: 3px;
			content: url("data:image/svg+xml,%3C%3Fxml version='1.0' encoding='iso-8859-1'%3F%3E%3Csvg version='1.1' id='Capa_1' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' x='0px' y='0px' width='20' height='15' viewBox='0 0 284.929 284.929' fill='#{ tool.encodecolor( $navigation__navigation-item__icon_color) }' style='enable-background:new 0 0 284.929 284.929;' xml:space='preserve'%3E%3Cg%3E%3Cpath d='M282.082,76.511l-14.274-14.273c-1.902-1.906-4.093-2.856-6.57-2.856c-2.471,0-4.661,0.95-6.563,2.856L142.466,174.441 L30.262,62.241c-1.903-1.906-4.093-2.856-6.567-2.856c-2.475,0-4.665,0.95-6.567,2.856L2.856,76.515C0.95,78.417,0,80.607,0,83.082 c0,2.473,0.953,4.663,2.856,6.565l133.043,133.046c1.902,1.903,4.093,2.854,6.567,2.854s4.661-0.951,6.562-2.854L282.082,89.647 c1.902-1.903,2.847-4.093,2.847-6.565C284.929,80.607,283.984,78.417,282.082,76.511z'/%3E%3C/g%3E%3C/svg%3E%0A");
		}

This works:

		&:after {
			margin-top: 3px;
			content: url("data:image/svg+xml,%3C%3Fxml version='1.0' encoding='iso-8859-1'%3F%3E%3Csvg version='1.1' id='Capa_1' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' x='0px' y='0px' width='20' height='15' viewBox='0 0 284.929 284.929' fill='#{ tool.encodecolor( $navigation__navigation-item__icon_color) }' style='enable-background:new 0 0 284.929 284.929' xml:space='preserve'%3E%3Cg%3E%3Cpath d='M282.082,76.511l-14.274-14.273c-1.902-1.906-4.093-2.856-6.57-2.856c-2.471,0-4.661,0.95-6.563,2.856L142.466,174.441 L30.262,62.241c-1.903-1.906-4.093-2.856-6.567-2.856c-2.475,0-4.665,0.95-6.567,2.856L2.856,76.515C0.95,78.417,0,80.607,0,83.082 c0,2.473,0.953,4.663,2.856,6.565l133.043,133.046c1.902,1.903,4.093,2.854,6.567,2.854s4.661-0.951,6.562-2.854L282.082,89.647 c1.902-1.903,2.847-4.093,2.847-6.565C284.929,80.607,283.984,78.417,282.082,76.511z'/%3E%3C/g%3E%3C/svg%3E%0A");
		}

So just changing style='enable-background:new 0 0 284.929 284.929;' to style='enable-background:new 0 0 284.929 284.929' fixed the issue for me.

I hope this helps, please let me know if I should report it somewhere else.

[strarsis]

@patrickwc: This sounds like this CSS parser-related issue: #21569

[wongjn]

The parser seems to error when trying to parse @layer rules.

[zaguiini]

I stumbled upon this issue again today... It's so frustrating.

Applying CSS rules that are perfectly valid break the block when the page initially loads.

Is there any activity here? Any way to help move this forward?

[strarsis]

@zaguiini: Gutenberg should use an iframe-based style isolation method in the near future:
#46212

My PR had the issue that it caused too much of an increase in overall Gutenberg size: #25514
Tree-shaking and other minification approaches had not been attempted yet.

[zaguiini]

Thanks, @strarsis!

So I'm guessing that the concern is mostly about bundle size increases. Are all functionalities of css-tree needed? I see that @youknowriad pointed out a 129KB increase in size but if we cherry-pick it might go down.

[strarsis]

@zaguiini: Yes, cherry-picking and tree-shaking and other optimizations for getting the size down would be great.
Most of it should not be used anyway.

[zaguiini]

Cool @strarsis, let me know if you need any help. If you don't have the bandwidth to work on it, I'd be happy to take over.

[strarsis]

@zaguiini: Yes! When you could find out how to reduce the bundle size the PR can be finally merged and most if not all parsing issues fixed (it will use a proper, well-tested CSS parser).
I assumed that tree-shaking should work, but apparently it does not include the css-tree library, hence no size reduction.

[skorasaurus]

other issues that this affects #46277 & #51601

[tcmulder]

Any movement on this? Currently I'm needing to enqueue a separate stylesheet (see comment) for all my container queries, while also adding my main styles the proper way through add_editor_style so I benefit from the .editor-styles-wrapper scoping class it adds for everything else. Container queries are such a perfect fit for block development since blocks can appear within many width contexts, which media queries can't accommodate since they're just looking at the entire viewport's width.

[strarsis]

@tcmulder: Yes, a PR is prepared (@zaguiini).

[zaguiini]

I pushed some more commits to the PR. Please check it again: #49521 (comment)

[strarsis]

@tcmulder, @kraftner, @skorasaurus, @Antonio-Laguna, @fabiankaegy, @patrickwc, @wongjn:
New editor styles transformation approach using PostCSS is now merged (many thanks @zaguiini)!

[youknowriad]

Thanks for the work everyone. Should we close this issue @strarsis

[strarsis]

@youknowriad: This should fix the issues linked above, so IMHO yes, this can be closed for now.