From 9d8ab5c22fc08b18f17825d6f105287c6010c4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 17 Sep 2020 12:20:08 +0200 Subject: [PATCH 1/9] Tell kses not to filter out link color prop --- lib/global-styles.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/global-styles.php b/lib/global-styles.php index 10180161342fb1..a023e7d0cfed0a 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -875,6 +875,18 @@ function gutenberg_experimental_global_styles_register_cpt() { register_post_type( 'wp_global_styles', $args ); } +/** + * Tell kses not to remove from the block markup (style attribute) + * the CSS variables the block editor may add. + */ +function gutenberg_experimental_global_styles_allow_css_variables( $allowed_attr ) { + return array_merge( + $allowed_attr, + [ '--wp--style--color--link' ], + ); +} + add_action( 'init', 'gutenberg_experimental_global_styles_register_cpt' ); add_filter( 'block_editor_settings', 'gutenberg_experimental_global_styles_settings' ); add_action( 'wp_enqueue_scripts', 'gutenberg_experimental_global_styles_enqueue_assets' ); +add_filter( 'safe_style_css', 'gutenberg_experimental_global_styles_allow_css_variables' ); From 8ed853ebb4382b99d0bfb59424015d5316aa4111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 17 Sep 2020 14:09:49 +0200 Subject: [PATCH 2/9] Tell kses to allow the values we pass to link color --- lib/global-styles.php | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/global-styles.php b/lib/global-styles.php index a023e7d0cfed0a..167c3af456f755 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -876,17 +876,42 @@ function gutenberg_experimental_global_styles_register_cpt() { } /** - * Tell kses not to remove from the block markup (style attribute) + * Tell kses not to remove from the block markup (from style attribute) * the CSS variables the block editor may add. + * + * @param array $allowed_attr List of allowed attributes. + * @return array Filtered result. */ -function gutenberg_experimental_global_styles_allow_css_variables( $allowed_attr ) { +function gutenberg_experimental_global_styles_allow_css_var_name( $allowed_attr ) { return array_merge( $allowed_attr, [ '--wp--style--color--link' ], ); } +/** + * Tell kses to allow certain values for the properties + * the block editor may add. + * + * @param boolean $allow_css Whether or not this rule should be allowed. + * @param string $css_test_string The CSS rule to process. + * @return boolean Filtered result. + */ +function gutenberg_experimental_global_styles_allow_css_var_value( $allow_css, $css_test_string ) { + $parts = explode( ':', $css_test_string, 2 ); + + if ( $parts[ 0 ] !== '--wp--style--color--link' ) { + return $allow_css; + } + + // The only value the block editor attaches to link color is + // var(--wp--preset--color--) + // so be specific in testing for that value. + return preg_match( '/var\(--wp--preset--color--.*\)/', $parts[1] ); +} + add_action( 'init', 'gutenberg_experimental_global_styles_register_cpt' ); add_filter( 'block_editor_settings', 'gutenberg_experimental_global_styles_settings' ); add_action( 'wp_enqueue_scripts', 'gutenberg_experimental_global_styles_enqueue_assets' ); -add_filter( 'safe_style_css', 'gutenberg_experimental_global_styles_allow_css_variables' ); +add_filter( 'safe_style_css', 'gutenberg_experimental_global_styles_allow_css_var_name' ); +add_filter( 'safecss_filter_attr_allow_css', 'gutenberg_experimental_global_styles_allow_css_var_value', 10, 2 ); From f36d75ccc415d734cf9377848bf62547f4bc3450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 17 Sep 2020 14:21:32 +0200 Subject: [PATCH 3/9] Make linter happy --- lib/global-styles.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/global-styles.php b/lib/global-styles.php index 167c3af456f755..09ada0fa149dee 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -885,7 +885,9 @@ function gutenberg_experimental_global_styles_register_cpt() { function gutenberg_experimental_global_styles_allow_css_var_name( $allowed_attr ) { return array_merge( $allowed_attr, - [ '--wp--style--color--link' ], + array( + '--wp--style--color--link', + ) ); } @@ -894,13 +896,13 @@ function gutenberg_experimental_global_styles_allow_css_var_name( $allowed_attr * the block editor may add. * * @param boolean $allow_css Whether or not this rule should be allowed. - * @param string $css_test_string The CSS rule to process. + * @param string $css_test_string The CSS rule to process. * @return boolean Filtered result. */ function gutenberg_experimental_global_styles_allow_css_var_value( $allow_css, $css_test_string ) { $parts = explode( ':', $css_test_string, 2 ); - if ( $parts[ 0 ] !== '--wp--style--color--link' ) { + if ( '--wp--style--color--link' !== $parts[ 0 ] ) { return $allow_css; } From efb04f6f2bc838ff1b4ca935fb7bd73093b66392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 17 Sep 2020 14:34:04 +0200 Subject: [PATCH 4/9] Provide more linter happiness --- lib/global-styles.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/global-styles.php b/lib/global-styles.php index 09ada0fa149dee..caadf66075b3ae 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -902,7 +902,7 @@ function gutenberg_experimental_global_styles_allow_css_var_name( $allowed_attr function gutenberg_experimental_global_styles_allow_css_var_value( $allow_css, $css_test_string ) { $parts = explode( ':', $css_test_string, 2 ); - if ( '--wp--style--color--link' !== $parts[ 0 ] ) { + if ( '--wp--style--color--link' !== $parts[0] ) { return $allow_css; } From 14d904ff00e9256ecf8f153ca1b19813f5b20a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Thu, 17 Sep 2020 17:49:00 +0200 Subject: [PATCH 5/9] Improve regular expression - match start and end of value characters - make sure color name can only have hyphen or alphanumeric chars --- lib/global-styles.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/global-styles.php b/lib/global-styles.php index caadf66075b3ae..daa60c44bfaaf2 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -907,9 +907,9 @@ function gutenberg_experimental_global_styles_allow_css_var_value( $allow_css, $ } // The only value the block editor attaches to link color is - // var(--wp--preset--color--) + // var(--wp--preset--color--) // so be specific in testing for that value. - return preg_match( '/var\(--wp--preset--color--.*\)/', $parts[1] ); + return preg_match( '/^var\(--wp--preset--color--[A-Za-z0-9-]*\)$/', $parts[1] ); } add_action( 'init', 'gutenberg_experimental_global_styles_register_cpt' ); From 0ab7205202c7c2edb4c85d653842ca2bf8f8ca59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 18 Sep 2020 09:48:04 +0200 Subject: [PATCH 6/9] Add e2e test --- .../__snapshots__/link-color.test.js.snap | 7 ++ .../specs/editor/various/link-color.test.js | 69 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 packages/e2e-tests/specs/editor/various/__snapshots__/link-color.test.js.snap create mode 100644 packages/e2e-tests/specs/editor/various/link-color.test.js diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/link-color.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/link-color.test.js.snap new file mode 100644 index 00000000000000..45427bbe1288dc --- /dev/null +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/link-color.test.js.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Link color control serializes markup properly 1`] = ` +" + +" +`; diff --git a/packages/e2e-tests/specs/editor/various/link-color.test.js b/packages/e2e-tests/specs/editor/various/link-color.test.js new file mode 100644 index 00000000000000..1f97be16d55a9d --- /dev/null +++ b/packages/e2e-tests/specs/editor/various/link-color.test.js @@ -0,0 +1,69 @@ +/** + * WordPress dependencies + */ +import { + clickBlockAppender, + createNewPost, + getEditedPostContent, + pressKeyWithModifier, + publishPost, +} from '@wordpress/e2e-test-utils'; + +describe( 'Link color', () => { + beforeEach( async () => { + await createNewPost(); + } ); + + const waitForAutoFocus = async () => { + await page.waitForFunction( + () => !! document.activeElement.closest( '.block-editor-url-input' ) + ); + }; + + it( 'control serializes markup properly', async () => { + // Create a block with some text + await clickBlockAppender(); + await page.keyboard.type( 'This is Gutenberg' ); + + // Select some text + await pressKeyWithModifier( 'shiftAlt', 'ArrowLeft' ); + + // Click on the Link button + await page.click( 'button[aria-label="Link"]' ); + + // Wait for the URL field to auto-focus + await waitForAutoFocus(); + + // Type a URL + await page.keyboard.type( 'https://wordpress.org/gutenberg' ); + + // Submit the link + await page.keyboard.press( 'Enter' ); + + // Simulate the theme has support for link color + await page.evaluate( () => { + wp.data.dispatch( 'core/block-editor' ).updateSettings( { + __experimentalFeatures: { + global: { + color: { link: true }, + }, + }, + } ); + } ); + + // Open color panel + await page.click('.block-editor-panel-color-gradient-settings .components-panel__body-toggle'); + + // Select first color + await page.click('.block-editor-panel-color-gradient-settings > .block-editor-color-gradient-control:last-child .components-circular-option-picker__option:first-child'); + + // Save post. + // We want to test that the link color control data + // persists after kses runs (kses filters out some markup, + // such as CSS variables within the style property). + await publishPost(); + + // Snapshot contains .has-link-color class and inline CSS variable + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); +} ); From 98d56c2a6523fbbed80cc51474cee03e2c1b1f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 18 Sep 2020 09:48:17 +0200 Subject: [PATCH 7/9] prettify --- .../e2e-tests/specs/editor/various/link-color.test.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/link-color.test.js b/packages/e2e-tests/specs/editor/various/link-color.test.js index 1f97be16d55a9d..f0747de84956d1 100644 --- a/packages/e2e-tests/specs/editor/various/link-color.test.js +++ b/packages/e2e-tests/specs/editor/various/link-color.test.js @@ -52,10 +52,14 @@ describe( 'Link color', () => { } ); // Open color panel - await page.click('.block-editor-panel-color-gradient-settings .components-panel__body-toggle'); + await page.click( + '.block-editor-panel-color-gradient-settings .components-panel__body-toggle' + ); // Select first color - await page.click('.block-editor-panel-color-gradient-settings > .block-editor-color-gradient-control:last-child .components-circular-option-picker__option:first-child'); + await page.click( + '.block-editor-panel-color-gradient-settings > .block-editor-color-gradient-control:last-child .components-circular-option-picker__option:first-child' + ); // Save post. // We want to test that the link color control data From 7b94a13c896245a566c67e07a363ed201687f520 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 18 Sep 2020 20:12:12 +0200 Subject: [PATCH 8/9] Pass through if value is a valid CSS color --- lib/global-styles.php | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/global-styles.php b/lib/global-styles.php index daa60c44bfaaf2..b88e9dd5e78f52 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -900,16 +900,24 @@ function gutenberg_experimental_global_styles_allow_css_var_name( $allowed_attr * @return boolean Filtered result. */ function gutenberg_experimental_global_styles_allow_css_var_value( $allow_css, $css_test_string ) { - $parts = explode( ':', $css_test_string, 2 ); + $parts = explode( ':', $css_test_string, 2 ); + $property_name = trim( $parts[0] ); + $property_value = trim( $parts[1] ); - if ( '--wp--style--color--link' !== $parts[0] ) { + // Pass through if we're not processing the link color property. + if ( '--wp--style--color--link' !== $property_name ) { return $allow_css; } - // The only value the block editor attaches to link color is - // var(--wp--preset--color--) - // so be specific in testing for that value. - return preg_match( '/^var\(--wp--preset--color--[A-Za-z0-9-]*\)$/', $parts[1] ); + // Pass through if $allow_css true. This means the link color has a valid color value + // (the user selected a custom color). + if ( $allow_css ) { + return $allow_css; + } + + // We want to be specific in testing that the value for link color + // matches this: var(--wp--preset--color--) + return preg_match( '/^var\(--wp--preset--color--[A-Za-z0-9-]*\)$/', $property_value ); } add_action( 'init', 'gutenberg_experimental_global_styles_register_cpt' ); From a9eb6ea5da00cdc26f35a8126223f9b3f350dd83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s?= Date: Fri, 18 Sep 2020 20:27:00 +0200 Subject: [PATCH 9/9] Make linter happy --- lib/global-styles.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/global-styles.php b/lib/global-styles.php index b88e9dd5e78f52..201caf0eff3327 100644 --- a/lib/global-styles.php +++ b/lib/global-styles.php @@ -916,7 +916,7 @@ function gutenberg_experimental_global_styles_allow_css_var_value( $allow_css, $ } // We want to be specific in testing that the value for link color - // matches this: var(--wp--preset--color--) + // matches this: var(--wp--preset--color--). return preg_match( '/^var\(--wp--preset--color--[A-Za-z0-9-]*\)$/', $property_value ); }