fix: Task 1 review round 2 - custom CSS builder-only, no settings on gvid/font references
S26: every css.* path is gated like S7 for users without unfiltered_html. S27: Divi ignores settings on global variable and theme font references, so any settings there are rejected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -35,10 +35,11 @@ final class AttrWriter
|
||||
private const URL_SEGMENT = '/url$/i';
|
||||
|
||||
/**
|
||||
* Raw HTML/CSS fields, and the ones Divi re-serializes whole pages for (ruling S7), at any
|
||||
* depth: module.advanced.html, module.advanced.html.htmlBefore, css.advanced.customCss.
|
||||
* Raw HTML/CSS fields, and the ones Divi re-serializes whole pages for (rulings S7/S26), by
|
||||
* path shape at any depth: module.advanced.html(.htmlBefore), module.decoration.attributes,
|
||||
* any free-form custom CSS path of any module (css, css.*, title.css.*, css.advanced.customCss).
|
||||
*/
|
||||
private const BUILDER_ONLY = '/(^|\.)(decoration\.attributes|advanced\.html|advanced\.customCss)(\.|$)/';
|
||||
private const BUILDER_ONLY = '/(^|\.)(decoration\.attributes|advanced\.html|advanced\.customCss|css)(\.|$)/';
|
||||
|
||||
public function __construct(
|
||||
private ModuleCatalog $catalog,
|
||||
|
||||
@@ -22,7 +22,7 @@ use stdClass;
|
||||
* Allowed names:
|
||||
* - type "color": a global color ID of the design system or a global variable of type "colors";
|
||||
* settings hue/saturation/lightness/opacity, numbers only.
|
||||
* - type "content": a non-color global variable or a theme font (settings DESIGN_SETTINGS);
|
||||
* - type "content": a non-color global variable or a theme font (no settings, DESIGN_SETTINGS, S27);
|
||||
* custom_meta_<key> with META_KEY (no leading "_", ruling S12; settings from the map entry of
|
||||
* that exact name, else META_SETTINGS); or a key of the dynamic content map.
|
||||
*
|
||||
@@ -59,8 +59,13 @@ final class VariableRef
|
||||
/** Settings of custom_meta_<key> without a map entry of its own. */
|
||||
public const META_SETTINGS = self::BEFORE_AFTER;
|
||||
|
||||
/** Settings of content references to global variables and theme fonts. */
|
||||
public const DESIGN_SETTINGS = self::BEFORE_AFTER;
|
||||
/**
|
||||
* Settings of content references to global variables and theme fonts: none (ruling S27).
|
||||
* Divi ignores them: theme fonts resolve to var(<name>) (DynamicData.php:300), and the gvid
|
||||
* option "doesn't have any settings" and renders without them
|
||||
* (DynamicContentGlobalVariableOptions.php:66-69, 155-161).
|
||||
*/
|
||||
public const DESIGN_SETTINGS = [];
|
||||
|
||||
private const BEFORE_AFTER = ['before' => 'richtext', 'after' => 'richtext'];
|
||||
|
||||
|
||||
@@ -263,6 +263,26 @@ final class AttrWriterTest extends TestCase
|
||||
ComposeFixture::attrWriter()->cleanValue('divi/button', 'module.decoration.attributes', ['attributes' => []]);
|
||||
}
|
||||
|
||||
/** Ruling S26: every free-form custom CSS path (css.*) is builder-only for editors, by path shape. */
|
||||
public function testCustomCssPathsNeedUnfilteredHtml(): void
|
||||
{
|
||||
$css = ['mainElement' => 'color:red;} body{display:none'];
|
||||
foreach (['css', 'css.desktop', 'css.advanced.customCss', 'css.advanced.mainElement', 'title.css'] as $path) {
|
||||
try {
|
||||
ComposeFixture::attrWriter()->cleanValue('divi/text', $path, $css);
|
||||
self::fail('Accepted for an editor: ' . $path);
|
||||
} catch (InvalidArgumentException $e) {
|
||||
self::assertStringContainsString('nur im Divi-Builder', $e->getMessage());
|
||||
}
|
||||
|
||||
self::assertSame($css, ComposeFixture::attrWriter(true)->cleanValue('divi/text', $path, $css));
|
||||
}
|
||||
|
||||
// Only a whole "css" segment counts.
|
||||
self::assertSame('x', ComposeFixture::attrWriter()->cleanValue('divi/text', 'module.advanced.cssClass', 'x'));
|
||||
self::assertSame('x', ComposeFixture::attrWriter()->cleanValue('divi/text', 'module.cssish', 'x'));
|
||||
}
|
||||
|
||||
/** Ruling S23: content.innerContent of unknown kind (defined only in Divi's JS) is rich text. */
|
||||
public function testUnknownContentInnerContentIsRichText(): void
|
||||
{
|
||||
|
||||
@@ -134,6 +134,8 @@ final class VariableRefTest extends TestCase
|
||||
'$variable({"type":"color","value":{"name":"gcid-a1b2;}</style><script>alert(1)</script>","settings":{}}})$',
|
||||
'Name',
|
||||
],
|
||||
'settings on a global variable' => [self::content('gvid-s1', '{"before":"<b>x</b>"}'), 'before'],
|
||||
'settings on a theme font' => [self::content('--et_global_heading_font', '{"after":""}'), 'after'],
|
||||
'unknown settings key' => [self::content('post_title', '{"before":"","foo":"1"}'), 'foo'],
|
||||
'setting the option lacks' => [self::content('post_featured_image', '{"before":"x"}'), 'before'],
|
||||
'enable_html' => [self::content('custom_meta_telefon', '{"before":"","after":"","enable_html":"on"}'), 'enable_html'],
|
||||
|
||||
Reference in New Issue
Block a user