Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wp_render_duotone_support() in > 5.9 prevents duotone filters being applied when loading page content with ajax. #39054

Open
slr1979 opened this issue Feb 24, 2022 · 14 comments
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended

Comments

@slr1979
Copy link

slr1979 commented Feb 24, 2022

Description

I am using a 3rd party theme which uses ajax/pjax page reloading. On upgrading from 5.8.3 to 5.9 and 5.9.1, I noticed that my images with duotone filters were no longer been applied when refreshing of content via ajax/pjax.

The wp_render_duotone_support() filter in > 5.9 does not allow any scope for use cases such as these and does not allow theme developers to control how/where the styles etc. are placed. The function assumes that a theme will being using hard pages reloads for every page. It would be great to find a solution that allowed more control over the placement of the Duotone styles etc. especially in the frontend. At this time I will be forced to use the solution below , stay on 5.8.3 or abandon the use of Duotone. Not ideal.

Step-by-step reproduction instructions

  1. Reload page content/main container using pjax/ajax on 5.8.3 - Duotone filters are successfully applied as relevant styles/filters/svg are within the main container which is reloaded via ajax/pjax.
  2. Reload page content/main container using pjax/ajax on > 5.9. - Duotone filters are not applied as relevant styles/filters/svg are placed in wp_footer which obviously does not get updated when reloading the main container using ajax/pjax .

Screenshots, screen recording, code snippet

So far, the only solution I have been able to find is to remove the wp_render_duotone_support() filter within my theme and replace it with a copy of the function from 5.8.3. I am unsure of the effect this may have on the block editor but it does not appear to cause any issues so far. As per other reports I am also forced to remove global styles and svg filters as they cause layout conflicts with the theme styles.

global $wp_version;
if ( version_compare( $wp_version, '5.9', '>=' ) ) {
	function remove_global_css() {
		remove_action( 'wp_enqueue_scripts', 'wp_enqueue_global_styles' );
		remove_action( 'wp_body_open', 'wp_global_styles_render_svg_filters' );
		remove_filter( 'render_block', 'wp_render_duotone_support', 10, 2 );
	}
	add_action('init', 'remove_global_css');

	function wp_render_duotone_support_58( $block_content, $block ) {
		$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );

		$duotone_support = false;
		if ( $block_type && property_exists( $block_type, 'supports' ) ) {
			$duotone_support = _wp_array_get( $block_type->supports, array( 'color', '__experimentalDuotone' ), false );
		}

		$has_duotone_attribute = isset( $block['attrs']['style']['color']['duotone'] );

		if (
			! $duotone_support ||
			! $has_duotone_attribute
		) {
			return $block_content;
		}

		$duotone_colors = $block['attrs']['style']['color']['duotone'];

		$duotone_values = array(
			'r' => array(),
			'g' => array(),
			'b' => array(),
		);
		foreach ( $duotone_colors as $color_str ) {
			$color = wp_tinycolor_string_to_rgb( $color_str );

			$duotone_values['r'][] = $color['r'] / 255;
			$duotone_values['g'][] = $color['g'] / 255;
			$duotone_values['b'][] = $color['b'] / 255;
		}

		$duotone_id = 'wp-duotone-filter-' . uniqid();

		$selectors        = explode( ',', $duotone_support );
		$selectors_scoped = array_map(
			function ( $selector ) use ( $duotone_id ) {
				return '.' . $duotone_id . ' ' . trim( $selector );
			},
			$selectors
		);
		$selectors_group  = implode( ', ', $selectors_scoped );

		ob_start();

		?>

		<style>
			<?php echo $selectors_group; ?> {
				filter: url( <?php echo esc_url( '#' . $duotone_id ); ?> );
			}
		</style>

		<svg
			xmlns:xlink="http://www.w3.org/1999/xlink"
			viewBox="0 0 0 0"
			width="0"
			height="0"
			focusable="false"
			role="none"
			style="visibility: hidden; position: absolute; left: -9999px; overflow: hidden;"
		>
			<defs>
				<filter id="<?php echo esc_attr( $duotone_id ); ?>">
					<feColorMatrix
						type="matrix"
						<?php // phpcs:disable Generic.WhiteSpace.DisallowSpaceIndent ?>
						values=".299 .587 .114 0 0
								.299 .587 .114 0 0
								.299 .587 .114 0 0
								0 0 0 1 0"
						<?php // phpcs:enable Generic.WhiteSpace.DisallowSpaceIndent ?>
					/>
					<feComponentTransfer color-interpolation-filters="sRGB" >
						<feFuncR type="table" tableValues="<?php echo esc_attr( implode( ' ', $duotone_values['r'] ) ); ?>" />
						<feFuncG type="table" tableValues="<?php echo esc_attr( implode( ' ', $duotone_values['g'] ) ); ?>" />
						<feFuncB type="table" tableValues="<?php echo esc_attr( implode( ' ', $duotone_values['b'] ) ); ?>" />
					</feComponentTransfer>
				</filter>
			</defs>
		</svg>

		<?php

		$duotone = ob_get_clean();

		// Like the layout hook, this assumes the hook only applies to blocks with a single wrapper.
		$content = preg_replace(
			'/' . preg_quote( 'class="', '/' ) . '/',
			'class="' . $duotone_id . ' ',
			$block_content,
			1
		);

		return $content . $duotone;
	}
	add_filter( 'render_block', 'wp_render_duotone_support_58', 10, 2 );
}

Environment info

WP 5.8.3 and WP 5.9.1, with Waveme (Flatfull)
All Browsers
Desktop with WIndows 10

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@slr1979
Copy link
Author

slr1979 commented Feb 25, 2022

Could anyone provide some feedback or other potential workarounds regarding this filter? I would appreciate it.

Placing styles in the footer may be good for FSE or standard WP themes but I can see implementations like this causing issues with ajax driven themes across the theme developer community. IMHO styles in the footer is not a normal design principle and creates headaches for theme developers who need inline styling within blocks.

@skorasaurus
Copy link
Member

@ajlende - could you sharing the design decision for moving it to the footer if you know?

@skorasaurus skorasaurus added the Needs Technical Feedback Needs testing from a developer perspective. label Mar 8, 2022
@ajlende
Copy link
Contributor

ajlende commented Mar 9, 2022

SVGs are not valid children of the <head> (MDN), so they have to appear somewhere in the <body>.

In 5.8.3, where it seemed to be working for you, they were included directly after the block, but this was causing issues with CSS selectors in themes. #30500, #32083

image

Later a bug in Safari was found with rendering in the footer, so it was moved to wp_body_open instead because that was the only place that it would render correctly. #36754

Safari

Screen Shot 2022-03-08 at 7 34 02 PM

Firefox/Chrome/Edge

Screen Shot 2022-03-08 at 7 33 44 PM

Then there was a bug with classic themes that didn't support wp_body_open and we wanted to keep the other generated styles (like the layout support used in the columns/group/etc. blocks) consistent, so it was moved back to wp_footer along with a repaint hack for Safari to fix the incorrect colors. #37954

So, @slr1979, I can't test Waveme because it isn't open source, but if the issue is with styles in the footer, do you also have issues with blocks that use layout support? (buttons, columns, comments-pagination, group, navigation, post-content, post-template, query, query-pagination, or social-links blocks)

@slr1979
Copy link
Author

slr1979 commented Mar 9, 2022

@ajlende Many thanks for the thorough explanation. It is very helpful. I am fairly new to Gutenberg and the block editor so please bear with me.

I do appear to have issues with at least button blocks too after checking today so that is not good. eg. center-aligned buttons end up aligned left after pjax refresh etc. The theme developer does use theme supports. I believe this means that core/global styles etc are loaded by default? At the moment the only way I can resolve this is to apply a css class of aligncenter to the button block itself which allows the global style to apply. Is there something I can do to solve this without having to resort to manually adding classes to the block?

// Add support for Block Styles.
add_theme_support( 'wp-block-styles' );

// Add support for full and wide align images.
add_theme_support( 'align-wide' );

// Add support for editor styles.
add_theme_support( 'editor-styles' );

// Add support for responsive embedded content.
add_theme_support( 'responsive-embeds' );

Alike the button blocks, the issue with duotone is only happening because the svg styles/filters have been moved into the footer outside the pjax containers.

With 5.8.3, the styles/filters for Duotone were loaded just after the image. This was great as pjax is updating one or several central containers within the body so every time a page is loaded with pjax, the new duotone inline css is within that container.

With < 5.9, as the svg styles/filter are rendered into the footer. As pjax/ajax page refeshing only updates specific containers within the body, styles that are outside these containers,( that would normally be rendered on a hard refresh ) are not rendered when refreshing a pjax container. I am sure you are well aware of this.

My issue is that the current implementation of the Duotone filters does not really accommodate ajax driven content in general. Normal layout blocks are handled fine with global styles in the head but elements that generate new styles from page to page (like Duotone images) can only be rendered if a page is loaded 'in full' NOT when only parts of the page are updated.

Obviously, this may appear to be an edge case but from my point of view, it would be amazing and maybe even essential to have a filter or solution that took this usage into account. It is still common for people to create single page applications that often require some inline styling for elements that update via pjax/ajax.

Do you think there will be a way to remedy this or should I stick to my fix from 5.8.3?

Your help is very much appreciated.

@ajlende
Copy link
Contributor

ajlende commented Mar 9, 2022

I do not appear to have any issue with blocks that use layout support but this is probably because the theme developer uses theme supports. I believe this means that core/global styles etc are loaded into the head by default?

The add_theme_support calls just add static styles, not the dynamic ones in question. But it looks like I was mistaken about layout. I missed this change #38750 that moved the dynamic layout styles into the head.

Obviously, this may appear to be an edge case but from my point of view, it would be amazing and maybe even essential to have a filter or solution that took this usage into account.

Not an edge case at all! Duotone and other styles like it should work in all of the contexts that themes typically render it in.

Do you think there will be a way to remedy this or should I stick to my fix from 5.8.3?

@andrewserong is working on a styles engine (#38167) to unify how styling is done. This seems to me like something that could be resolved along with those efforts. The styles engine should consider SVGs that need to be rendered in the body along with the CSS that can be rendered wherever.

@slr1979
Copy link
Author

slr1979 commented Mar 9, 2022

@ajlende. Please re-read my previous comment as I edited it just minutes before your reply! I am having issues with some of my layout and blocks after all. It appears that some of the styling that was applied before I updated to 5.9.1 is in the individual blocks as classes. If I remove those 'hard-coded' classes and use the dynamic layout styles etc, it all goes horribly wrong.

Do you anticipate these modifications taking long ? I am due to go into production in the next week or so. I can get around it in my Gutenberg pages by using my 5.8.3 based filter (from my first comment) and putting classes into each block but it would be great to be able to use the editor in the way it was meant to be used and get proper ajax compatibility so to speak.

It is great to know I am not imaging these issues or jumping the gun thinking it should work as I hope! Do get back to me with any further thoughts.

@slr1979
Copy link
Author

slr1979 commented Mar 9, 2022

@ajlende and yes, social icons have gone wrong too.

@ajlende
Copy link
Contributor

ajlende commented Mar 9, 2022

Do you anticipate these modifications taking long?

Yes, probably, since this is affecting a number of areas in a rather fundamental way. It's good that you've found a workaround, but it's unfortunate that you may need to use it for a while until this is resolved.

Social icons have gone wrong too.

It seems like it may be related to #35376 as well. Do the social icons look like this?

Screen Shot 2021-10-06 at 12 17 21 pm

@ajlende ajlende added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended CSS Styling Related to editor and front end styles, CSS-specific issues. and removed Needs Technical Feedback Needs testing from a developer perspective. labels Mar 9, 2022
@slr1979
Copy link
Author

slr1979 commented Mar 9, 2022

@ajlende Pretty much...
social-icons pre ajax
social-icons post ajax

Can get them to stick with this in child stylesheet

.wp-block-social-links{
	display: flex;
       gap: 0.5em;
       flex-wrap: nowrap;
      align-items: center;
}

.wp-block-social-links.alignright{
	float:right !important;
}

Obviously, again not ideal for quick page edits or design changes but would rather have 5.9.1 with up-to-date security fixes and a slightly dodgy Gutenberg. I know you guys are trying really hard to cover an awful lot of bases but in some respects, it almost feels that it would be best for WP to backport to the version of Gutenberg pre 5.9! I'm sure it will come good in the end though but at the moment it feels slightly frustrating. It was all going so well on my dev until 5.9 ;-)

@slr1979
Copy link
Author

slr1979 commented Jun 9, 2022

Could @ajlende or anyone update me on the progress of the style engine/fixes in relationship to asynchronous contexts? It has been 4 months since I first reported this and I would appreciate further info.

I have just updated to WP 6.0 on my staging site and I am now experiencing similar issues with the block gallery even when reenabling global styles and layout support in the code below.

global $wp_version;
if ( version_compare( $wp_version, '5.9', '>=' ) ) {
	
	function remove_global_css() {
		// remove fluff created by global css & duotone svg filters
		//remove_action( 'wp_enqueue_scripts', 'wp_enqueue_global_styles' );
		//remove_filter( 'render_block', 'wp_render_layout_support_flag', 10, 2 );
		remove_action( 'wp_body_open', 'wp_global_styles_render_svg_filters' );
		
		// remove duotone support and replace with version 5.8 filter
		remove_filter( 'render_block', 'wp_render_duotone_support', 10, 2 );
		add_filter( 'render_block', 'wp_render_duotone_support_58', 10, 2 );
	}
	add_action('init', 'remove_global_css');

	function wp_render_duotone_support_58( $block_content, $block ) {
		$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );

		$duotone_support = false;
		if ( $block_type && property_exists( $block_type, 'supports' ) ) {
			$duotone_support = _wp_array_get( $block_type->supports, array( 'color', '__experimentalDuotone' ), false );
		}

		$has_duotone_attribute = isset( $block['attrs']['style']['color']['duotone'] );

		if (
			! $duotone_support ||
			! $has_duotone_attribute
		) {
			return $block_content;
		}

		$duotone_colors = $block['attrs']['style']['color']['duotone'];

		$duotone_values = array(
			'r' => array(),
			'g' => array(),
			'b' => array(),
		);
		foreach ( $duotone_colors as $color_str ) {
			$color = wp_tinycolor_string_to_rgb( $color_str );

			$duotone_values['r'][] = $color['r'] / 255;
			$duotone_values['g'][] = $color['g'] / 255;
			$duotone_values['b'][] = $color['b'] / 255;
		}

		$duotone_id = 'wp-duotone-filter-' . uniqid();

		$selectors        = explode( ',', $duotone_support );
		$selectors_scoped = array_map(
			function ( $selector ) use ( $duotone_id ) {
				return '.' . $duotone_id . ' ' . trim( $selector );
			},
			$selectors
		);
		$selectors_group  = implode( ', ', $selectors_scoped );

		ob_start();

		?>

		<style>
			<?php echo $selectors_group; ?> {
				filter: url( <?php echo esc_url( '#' . $duotone_id ); ?> );
			}
		</style>

		<svg
			xmlns:xlink="http://www.w3.org/1999/xlink"
			viewBox="0 0 0 0"
			width="0"
			height="0"
			focusable="false"
			role="none"
			style="visibility: hidden; position: absolute; left: -9999px; overflow: hidden;"
		>
			<defs>
				<filter id="<?php echo esc_attr( $duotone_id ); ?>">
					<feColorMatrix
						type="matrix"
						<?php // phpcs:disable Generic.WhiteSpace.DisallowSpaceIndent ?>
						values=".299 .587 .114 0 0
								.299 .587 .114 0 0
								.299 .587 .114 0 0
								0 0 0 1 0"
						<?php // phpcs:enable Generic.WhiteSpace.DisallowSpaceIndent ?>
					/>
					<feComponentTransfer color-interpolation-filters="sRGB" >
						<feFuncR type="table" tableValues="<?php echo esc_attr( implode( ' ', $duotone_values['r'] ) ); ?>" />
						<feFuncG type="table" tableValues="<?php echo esc_attr( implode( ' ', $duotone_values['g'] ) ); ?>" />
						<feFuncB type="table" tableValues="<?php echo esc_attr( implode( ' ', $duotone_values['b'] ) ); ?>" />
					</feComponentTransfer>
				</filter>
			</defs>
		</svg>

		<?php

		$duotone = ob_get_clean();

		// Like the layout hook, this assumes the hook only applies to blocks with a single wrapper.
		$content = preg_replace(
			'/' . preg_quote( 'class="', '/' ) . '/',
			'class="' . $duotone_id . ' ',
			$block_content,
			1
		);

		return $content . $duotone;
	}
}

BUT AGAIN, as all styles etc are rendered outside of the ajax container, the problem now also occurs when using the block gallery when the styles have not been previously rendered via a hard page refresh or direct load. There must be a way to inline styles for async contexts that doesn't mean reinventing the wheel, surely?

I would be grateful if someone could reference this again in relevant issues and maybe offer any intermediate solutions apart from abandoning Gutenberg, which I actually quite like....sometimes 😂

@ajlende
Copy link
Contributor

ajlende commented Jun 9, 2022

@slr1979 It looks like refactoring duotone to work in the style engine is on the checklist in #38167.

Maybe @andrewserong can share some details?

@slr1979
Copy link
Author

slr1979 commented Jun 23, 2022

@ajlende @andrewserong I have just completed a workaround for the issue with Gutenberg styles and duotone when using asynchronous page loading - in my case pjax . This approach is working well with WP6.0 #38167 #38681

I thought I would share it with you as it may be some help with the style engine? Obviously, this code is based on using pjax but I believe that some amazing, experienced js dev may be able to take my slightly long-winded approach and make it into something more useful and generic in WP perhaps? I am a self-taught coder/dev so apologies if this all looks a little naive.

As you will see this code runs on the success event of pjax so could potentially be adapted for use native js/react or any other asynchronous method. The idea is to search for all styles and svgs below the main/refreshed container and reload them below existing elements hopefully avoiding conflicts with theme styles etc. The only caveat is that I have had to make an ajax call to get the full page html as obviously pjax only updates parts of the page.

$(document).on('pjax:success', function() {
		$('html').addClass('duotone-animate');
		// reload footer styles && svgs
		$.ajax({ 
			url: window.location.href, 
			dataType: 'html',
			success: function(response) { 
				var data = $(response);
				var css_arr = [];
				var svg_arr = [];
				data.each(function(i, v){
					var after_content = $(v).prevAll().is('#content');
						
					if(after_content){
						var css = $(v).text(),
						style = $(v).is('style'),
						svg = $(v).find('filter[id*="duotone"]').parent().parent().prop("outerHTML");
						
						if(svg){
							svg_arr.push(svg);
						}
							
						if(style){
							css_arr.push(css);
						}		
					}
				});
				var new_styles = css_arr.join(" ");
				var new_svgs = svg_arr.join(" ");
				
				$('#css-after-content').remove();
				$('filter[id*="duotone"]').parent().parent().remove();
				$('<style id="css-after-content">'+ new_styles +'</style>').insertAfter( $('style').last() );
				$(new_svgs).insertBefore( '#css-after-content' );
				
				$('html').removeClass('duotone-animate');	
			}
		});
});

The adding/removing of the duotone-animate class is simply a css transition that I use to elimnate the flash when the duotone svg filters and css updates.

div[class*="wp-duotone-"] img:not(.duotone-animate div[class*="wp-duotone-"] img){
	-webkit-animation: fadein 0.5s linear;
	animation: fadein 0.5s linear;
}

.duotone-animate div[class*="wp-duotone-"] img{
	opacity:0
}

I have also been able to re-enable all the actions that were removed to allow duotone to work except for wp_global_styles_render_svg_filters which appears to be unnecessary fluff in my use case.

I hope you and the wp/gutenberg team may find something useful in this that could help with asynchronous requests.

@slr1979
Copy link
Author

slr1979 commented Jun 24, 2022

@ajlende @andrewserong Now I'll try that again with less excitement (i.e checking it again before posting), less code, and working as it should. #38167 #38681

$(document).on('pjax:success', function() {
	$('html').addClass('duotone-animate');
	// reload footer styles && svgs
	$.ajax({ 
		url: window.location.href, 
		dataType: 'html',
		success: function(response) { 
			var html = $(response),
			el_after_content = html.filter('#content').nextAll(),
			styles = el_after_content.filter('style').text(),
			svgs = el_after_content.find('filter[id*="duotone"]');
				
			//console.log(el_after_content);
			//console.log(styles);	
			//console.log(svgs);
				
			$('filter[id*="duotone"]').parent().parent().remove();
			$('#after-content-css').remove();
				
			if(styles){
				$('#content').nextAll('style').remove();
				$('<style id="after-content-css">' + styles +'</style>').insertBefore( $('#content').nextAll('script').first());
			}
					
			if(svgs){
				$(svgs).each(function(i, el){
					var svg = $(el).parent().parent().prop("outerHTML");
					$(svg).insertBefore('#after-content-css');
				});
			}

			$('html').removeClass('duotone-animate');	
		}
	});
});

@andrewserong
Copy link
Contributor

andrewserong commented Jun 27, 2022

Thanks for sharing the code snippet @slr1979, glad to hear you could come up with a workaround for your use case!

It looks like refactoring duotone to work in the style engine is on the checklist in #38167. Maybe @andrewserong can share some details?

Apologies, I think I must have missed this earlier ping while I was AFK that week. There's been some recent status updates on the style engine tracking issue and a recent blog post communicating where the focus on is currently on improvements to how styles are output in Gutenberg: https://make.wordpress.org/core/2022/06/24/block-editor-styles-initiatives-and-goals/

My current focus is on improving how the Layout block support works, which will have the side effect of resolving most instances of layout styles rendering incorrectly in asynchronous contexts, if the base page view contains some base Layout CSS generated by global styles (the current work in progress is in #35376). Unfortunately, I don't think Duotone will benefit much from that particularly fix, as that fix uses classnames to link content in async contexts to styles that already exist in the first page render.

At the moment, as far as I know, Duotone support in async contexts is lower down the list of priorities (it's an item on the checklist of the style engine tracking issue, but I don't think anyone's picked it up), as Layout styles are more commonly used and will be a higher impact fix in the shorter-term. But don't let that stop anyone from digging in to come up with a solution, if you have the time to explore it! From what I understand from the earlier comments in this issue, the Duotone feature is quite unique with where SVG elements need to be placed in markup, so I imagine it'll need some rethinking to come up with a reliable solution.

@ajlende did you have a particular idea in mind for how the style engine work intersects here? If you have the time, it might be helpful to list out how we think the block support should work ideally, and break down the tasks to get there from here? (I followed a similar approach of opening up some experimental PRs to dig into the Layout support, which helped inform the current refactor — most of the other block supports have been fairly straightforward, but Layout and Duotone sure have their complexities! 😅).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants