Make WordPress Core

Opened 6 years ago

Closed 4 years ago

#46498 closed enhancement (fixed)

Block style attribute issue, when using Custom properties/CSS variables

Reported by: olafklejnstrupjensen's profile olafklejnstrupjensen Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch
Focuses: ui, css, docs, administration Cc:

Description

First a little background on the issue

We use Custom properties, also referred to as CSS variables, when creating themes for our clients. This helps us define various color schemes quickly, for each new client and makes our development of new project much quicker.

In WP, we then add theme support for the editor-color-palette setting and add our CSS variables inside the array.

Example

<?php
add_theme_support(
        'editor-color-palette',
        [
                [
                        'name'  => 'color 1',
                        'color' => 'var(--color-1)',
                ],
                [
                        'name'  => 'color 2',
                        'color' => 'var(--color-2)',
                ],
        ]
);

This adds our color scheme to the various block elements, that use the PanelColorSettings, so that our clients can quickly access the colors in their color schemes.

The issue

This works great with admins and super admins, that have the unfiltered html capability, as they are allowed to add any inline style. However, if the user does not have that capability, then the css part is stripped, as part of the safe_style_css function. (https://core.trac.wordpress.org/browser/tags/5.1.1/src/wp-includes/kses.php#L2214)

For our setup, this is not very practical, as all of that functionality is hardcoded and we can not override it, except by giving all contributing user roles, the unfiltered html capability.

But a more unexpected issue is that if an admin with the unfiltered html capability authors a post and chooses a background color for a core paragraph block, the block gets the right style attributes, even with the CSS variable. Later on, a user without that capability adds a correction to that page. When the user save their changes, the style attribute containing the CSS variable, var(--color-1), gets that part stripped. However the block attribute is not removed and as it remains in the block attribute, the block displays correctly in the block editor. The next time an administrator with the unfiltered html capability edits the post, the core paragraph block creates a new paragraph and wraps the existing paragraph inside the new one, thus breaking the block in the block editor.

Fixes

There are a multiple ways of fixing this.

  1. We stop using CSS variables in our editor-color-palette array. However these CSS variable are part of modern web development and very practical.
  2. The block editor also strips attributes that are deemed unsafe in accordance with the users role and capabilites
  3. The safecss_filter_attr function gets another hardcoded exception for CSS variables, var(), that mimics the way the url() exception works.
  4. A hook is introduced in the safecss_filter_attr function allowing developers a chance to override the allowed style attribute values.

I would like to hear if others are affected by this issue and the opinions regarding the above practice and if there are suggestions for best practices for these cases.

I know that this issue is mainly related to the use of Gutenberg, however the safe_style_css that strips the CSS values is part of the core of WordPress, so I think this is more an issue for WordPress.

I am also in doubt if this qualifies as a bug, as the behaviour could be seen as expected behaviour, so some might see it as a feature request or enhancement, however the above example breaks the block editor and could then be seen as a bug.

Change History (6)

#1 @sabernhardt
5 years ago

  • Component changed from General to Editor
  • Focuses coding-standards removed
  • Type changed from defect (bug) to enhancement

@olafklejnstrupjensen Hi and thanks for the report!

Yes, this would be more of an enhancement. The theme support documentation only mentions using the hexadecimal color code for the editor color palette.

But I think more theme developers will want to use CSS variables, especially once they can stop supporting Internet Explorer.

Though this might fit under the Themes component, some other tickets involving safe_style_css (#42729 and #47281) were under the Editor component.

#2 @anonymized_7837873
4 years ago

  • Keywords needs-patch added

Great ticket. We have the same issue. The style attributes should support using style="width: var(--scale)" AND setting style="--scale:80%;" custom properties.

Using a custom property is great because you can avoid hardcoding values into the html, meaning full control of the style values remains in the css files.

Setting a custom property is really useful if you want to use a value from a Gutenberg editor control in your css without writing a lot of classes to handle all the available values from that control. That's really great if the control has a lot of possible (or arbitrary) values.

As far as I can see this is going to require adding a new type, much like the url and gradient types are handled. What can we do to move this forward? I could have a go at a patch if I can find the time.

#3 @noisysocks
4 years ago

  • Focuses css added

#4 @aristath
4 years ago

  • Keywords has-patch added; needs-patch removed

Patch https://github.com/WordPress/wordpress-develop/pull/1260 will take care of this one as well as #46197

#5 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.8
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#6 @SergeyBiryukov
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 50923:

KSES: Allow calc() and var() values to be used in inline CSS.

Props aristath, displaynone, joyously, olafklejnstrupjensen, sabernhardt, jamesbonham, poena.
Fixes #46197, #46498.

Note: See TracTickets for help on using tickets.