Make WordPress Core

Opened 14 months ago

Closed 13 months ago

Last modified 9 months ago

#57780 closed defect (bug) (fixed)

wp_kses: Add filter to css_url_data_types

Reported by: scruffian's profile scruffian Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-testing-info has-screenshots gutenberg-merge add-to-field-guide
Focuses: Cc:

Description (last modified by hellofromTonya)

Since filter can also refer to urls (see https://developer.mozilla.org/en-US/docs/Web/CSS/url) then we should add it to the css_url_data_types array.

PR here: https://github.com/WordPress/wordpress-develop/pull/4108

Reference:

.

Change History (10)

This ticket was mentioned in PR #4108 on WordPress/wordpress-develop by @scruffian.


14 months ago
#1

  • Keywords has-patch added

Since filter can also refer to urls (see https://developer.mozilla.org/en-US/docs/Web/CSS/url) then we should add it to the css_url_data_types array.

Trac ticket: https://core.trac.wordpress.org/ticket/57780#ticket

#2 @jeryj
14 months ago

Is it worth adding the other properties that url can be used with?

The url() function can be included as a value for background, background-image, border, border-image, border-image-source, content, cursor, filter, list-style, list-style-image, mask, mask-image, offset-path, src as part of a @font-face block, and @counter-style/symbol

I'm not sure what the performance overhead would be to add more to the array here if they're going to be mostly unused. If any of them are needed they can certainly be added in follow-up PRs as needed. I'm mostly curious to learn more about context here.

#3 @ironprogrammer
13 months ago

  • Keywords has-testing-info has-screenshots added

For additional context, this update is required for core support of this duotone PR: https://github.com/WordPress/gutenberg/pull/48281.

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/4108

Steps to Test

  1. Make sure that the Gutenberg plugin is disabled.
  2. Install the test mu-plugin located here.
  3. Confirm that the displayed image is in purple/yellow duotone, and inspect the markup to verify styles applied.

Expected Results

  • ✅ The displayed image should be in purple/yellow duotone.
  • ✅ The image should not be blurred.

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 12.6.3
  • Browser: Safari 16.3
  • Server: nginx/1.23.3
  • PHP: 7.4.33
  • WordPress: 6.2-beta5-55481-src
  • Theme: twentytwentythree v1.1
  • Active Plugins:
    • modern-images-wp v1.1.0
    • trac-57780 (mu-plugin noted above)

Actual Results

  • ✅ The kitten image was displayed with the purple/yellow duotone effect.
  • ✅ Image was not blurred (i.e. filter: blur(5px) was not allowed through wp_kses()).

Supplemental Artifacts

Before Patch After Patch
https://cldup.com/-y9y-jSvKI.thumb.jpg https://cldup.com/qUimESjQup.thumb.jpg

#4 follow-up: @azaozz
13 months ago

  • Milestone changed from Awaiting Review to 6.3

The CSS filter seems "safe" at first look.

#5 @hellofromTonya
13 months ago

  • Description modified (diff)
  • Keywords gutenberg-merge added

#6 in reply to: ↑ 4 ; follow-up: @SergeyBiryukov
13 months ago

Replying to azaozz:

The CSS filter seems "safe" at first look.

FWIW, filter was already added to safecss_filter_attr() a while ago in [52049] / #54336.

This PR appears to be about specifically using filter with a URL:

filter: url( my-file.svg#svg-blur );

#7 @SergeyBiryukov
13 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 55564:

KSES: Allow filter property to accept a URL in safecss_filter_attr().

CSS filters can accept url() as a reference to an SVG filter element:

filter: url( file.svg#filter-element-id );

This commit allows for that syntax to be used in inline CSS.

Original PR from Gutenberg repository:

References:

Follow-up to [44136], [52049].

Props scruffian, jeryj, ironprogrammer, azaozz, hellofromTonya, SergeyBiryukov.
Fixes #57780.

@SergeyBiryukov commented on PR #4108:


13 months ago
#8

Thanks for the PR! Merged in r55564.

#9 in reply to: ↑ 6 @azaozz
13 months ago

Replying to SergeyBiryukov:

This PR appears to be about specifically using filter with a URL:

filter: url( my-file.svg#svg-blur );

Yea, sorry, should have explained better. Was wondering what happens to non-image files (SVG, etc.) when loaded from CSS, and if that may need more consideration.

#10 @milana_cap
9 months ago

  • Keywords add-to-field-guide added
Note: See TracTickets for help on using tickets.