WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 2 months ago

#37134 new enhancement

Allow filtering of safecss_filter_attr

Reported by: paulschreiber Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.6
Component: Formatting Keywords: kses has-patch
Focuses: Cc:

Description

It would be helpful if sites could customize the allowed CSS. This would work around #24157 (which is 3+ years old).

In kses.php, we could solve this by filtering safecss_filter_attr. Diff attached.

Attachments (4)

37134.diff (443 bytes) - added by paulschreiber 4 years ago.
37134.2.diff (435 bytes) - added by paulschreiber 4 years ago.
37134.3.diff (779 bytes) - added by bartekcholewa 2 years ago.
safecss_filter_attr - filter to unallowed chars
37134.4.diff (1.0 KB) - added by miinasikk 10 months ago.
Update patch based on changes. Add CSS value as filter param.

Download all attachments as: .zip

Change History (17)

@paulschreiber
4 years ago

#1 @swissspidy
4 years ago

  • Component changed from General to Formatting
  • Keywords kses added

Filters with the same name as the function are usually inside that function, not wrapped around it.

#2 @rmccue
4 years ago

This seems already possible via the safe_style_css filter, which controls the allowed properties in the CSS. It would potentially be nice to have more granular control though, allowing only specific values for fields e.g.

#3 @paulschreiber
4 years ago

@rmccue The safe_style_css filter won't work in this case, because, as described in #24157 safecss_filter_attr rejects any CSS with parentheses. Meaning <p style="color: rgb(100,100,100,0.5)"> gets rejected even though the color attribute is already whitelisted (see safecss_filter_attr() in kses.php around line 1714).

#4 @rmccue
4 years ago

Got it, so this needs to be a short-circuit hook at the start of the function then; probably pre_safe_style_css for the name.

#5 @paulschreiber
4 years ago

Okay. I uploaded a new diff which uses pre_safe_style_css as the filter name.

#6 @paulschreiber
4 years ago

  • Keywords has-patch added

#7 @paulschreiber
4 years ago

@rmccue?

#8 @paulschreiber
4 years ago

Can we include this in 4.7?

#9 @swissspidy
4 years ago

37134.2.diff would need inline docs as per the PHP Documentation Standards.

Edit: Also, the original $value should probably be passed to the filter as the second argument. Otherwise it seems a bit pointless.

Last edited 4 years ago by swissspidy (previous) (diff)

@bartekcholewa
2 years ago

safecss_filter_attr - filter to unallowed chars

#10 @bartekcholewa
2 years ago

We could simply add filter to pattern in preg_match, i've added 36134.3.fdiff

@miinasikk
10 months ago

Update patch based on changes. Add CSS value as filter param.

#11 @paulschreiber
2 months ago

Can someone review this? It's been sitting around for 3 years.

#12 @adamsilverstein
2 months ago

I'll take a look @paulschreiber - a filter here seems sensible and unlikely to introduce backwards compatibility issues. We already have similar filters elsewhere in kses.php to enable developer flexibility.

Does this get you what you need to solve the issue in https://core.trac.wordpress.org/ticket/24157? Does that issue really need solving with a new regex, or would adding this filter here also solve that issue essentially?

#13 @paulschreiber
2 months ago

We should fix #24157 separately — rgba() / rgb() should work for everyone, not just developers.

Note: See TracTickets for help on using tickets.