Allow more complex contentWidth & wideWidth values #31740
Conversation
aristath
added
the
[Feature] Themes
|
Size Change: -13.9 kB (-1%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
|
Feels like this could have some security impact. Should |
Not really...
The issue here is that |
|
I only tested with the Q theme and the PR solves the issues with the missing widths in the CSS. The output for a group with |
Hi @aristath, gutenberg/lib/global-styles.php Line 256 in c8512db It seems something that will require a complex parsing but sooner or later we should probably do it to allow users without full output permissions to use things like a calc or clamp in their block styles. |
|
If we want to get this in core, I submitted a patch in WordPress/wordpress-develop#1260 If we can't make that patch on time for 5.8, then I added |
There was a problem hiding this comment.
Hi @aristath,
I tested your last updates and it mitigates part of the security issue but we still allow something that normally a non-privileged user would not be able to do like set z-index or CSS variables:
<!-- wp:group {"className":"wewew","layout":{"contentSize":"500px; z-index: 4; \u002d\u002dwp\u002d\u002dprivate\u002d\u002dvar: red"}} -->
<div class="wp-block-group wewew"><!-- wp:paragraph -->
<p>test</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
Maybe we can mitigate this issue by also removing the char ";" from the strings? It would force the css rule to always be interpreted as max-width.
This would be a temporary solution until an update to the existing CSS mechanism in the form of a core patch or a filter in Gutenberg is ready. |
Good catch! Added a tweak using |
|
I had a conflict cherry-picking this into release/10.6 I solved it but I'd appreciate a sanity check :) |
Description
contentWidthandwideWidthwill not always be as simple as800pxor70em. In some cases, more complex values are used to accomplish different results. In the Q theme for example, I'm using adaptive/responsive typography and the content-width adapts to the values of that.So in the
theme.jsonfile I have this:{ "settings": { "layout": { "contentSize": "calc((var(--wp--custom--typo--root-size) * 1px + var(--wp--custom--typo--adaptive-ratio) * 1vw) * var(--wp--custom--typo--line-width) / 2)", "wideSize": "calc(1.5 * (var(--wp--custom--typo--root-size) * 1px + var(--wp--custom--typo--adaptive-ratio) * 1vw) * var(--wp--custom--typo--line-width) / 2)" }, "custom": { "typo": { "rootSize": 16, "adaptiveRatio": 0.8, "scale": 1.333, "lineHeight": 1.55, "lineWidth": 74 } } } }Now, if with these settings I add a group and set it to inherit the default layout, then this is the CSS that gets added on the frontend:
so, the
max-widthdefinitions are missing.This PR fixes the issue by removing the
safecss_filter_attrcall which can't handle values including complexcalc().In the process of doing that, the PR also simplifies the implementation a bit by setting the
$stylePHP var instead of using an output buffer.How has this been tested?
Tested with the TT1-blocks & Q themes.
Checklist:
*.native.jsfiles for terms that need renaming or removal).