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

Fix duotone render in non-fse themes #37954

Merged
merged 6 commits into from
Jan 17, 2022
Merged

Fix duotone render in non-fse themes #37954

merged 6 commits into from
Jan 17, 2022

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Jan 13, 2022

Description

Fixes #37598

It seems that the render_block filter is being called after the wp_body_open hook in non-fse themes, so this change will fall back on wp_footer if it has already been called.

We switched from wp_footer to wp_body_open in #36754 because Safari renders blocks incorrectly when the SVG filter comes after the content that it is filtering. Switching back unfortunately means that duotone rendering will be incorrect for non-fse themes when viewed from Safari.

How has this been tested?

  1. Activate a non-fse theme like Blank Canvas.
  2. In a post, add an image with a duotone filter.
  3. Publish and view the post to see the filter applied.

Screenshots

Using the Blank Canvas theme using the one of the default duotone filters applied to the block

Before:
image does not render at all

After:
duotone filter is correctly applied to the image

Types of changes

Bug Fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@ajlende ajlende added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block labels Jan 13, 2022
@ajlende ajlende self-assigned this Jan 13, 2022
@youknowriad
Copy link
Contributor

Is there a way to force a repaint in safari (since it seems that fixes the issue according to the link PR)

@ajlende
Copy link
Contributor Author

ajlende commented Jan 13, 2022

Is there a way to force a repaint in safari (since it seems that fixes the issue according to the link PR)

It's going to get messy if we do. We'd have to do a user agent check and add a script for each duotone filter to hide and show the filtered content in Safari according to a blog post that I found.

@youknowriad
Copy link
Contributor

My main concern with wp_body_open is that we're creating inconsistencies between the different ways we render server side styles. Layout block support use wp_footer right now and ideally we could share code between all these server side styles to enqueue the styles in the exact same way.

Anyway, my comments shouldn't block you here but as part of the discussion here #37495 I think ultimately enqueue this kind of styles should be consistent.

@ajlende
Copy link
Contributor Author

ajlende commented Jan 14, 2022

@youknowriad, I gave the repaint a go in Safari. It's a little messy, and I sometimes see the images flash, but it seems to work.

ideally we could share code between all these server side styles to enqueue the styles in the exact same way.

It would be nice to have a better system for adding things like this—I keep thinking about other SVG filter opportunities because there's a lot of things that they can do, but it's kind of clunky to add them right now.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like the best fix I can think of. Thanks

@youknowriad youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jan 17, 2022
@ajlende ajlende merged commit 3715fa4 into trunk Jan 17, 2022
@ajlende ajlende deleted the fix/duotone-non-fse branch January 17, 2022 13:37
@github-actions github-actions bot added this to the Gutenberg 12.5 milestone Jan 17, 2022
@noisysocks noisysocks added Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release and removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jan 17, 2022
@noisysocks
Copy link
Member

I can't backport this PR as it depends on #36236 which was not backported into 5.9. Ordinarily I would backport both PRs but #36236 contains quite a bit of new PHP code and we are very late into the RC period.

I've put the Backport to WP Minor Release on both PRs so that both fixes are included in 5.9.1 which should follow 5.9 very quickly.

@mcsf mcsf added the Browser Issues Issues or PRs that are related to browser specific problems label Jan 21, 2022
@Mamaduka Mamaduka removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 16, 2022
Mamaduka pushed a commit that referenced this pull request Feb 16, 2022
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Feb 17, 2022
This changeset is a backport for the following Gutenberg PRs:

- Fix duotone theme cache: WordPress/gutenberg#36236
- Fix duotone render in non-fse themes gutenberg: WordPress/gutenberg#37954
- Duotone: Allow users to specify custom filters gutenberg: WordPress/gutenberg#38442

Props oandregal, scruffian, Mamaduka.
Merges [52757] to the 5.9 branch.
See #55179.


git-svn-id: https://develop.svn.wordpress.org/branches/5.9@52759 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Feb 17, 2022
This changeset is a backport for the following Gutenberg PRs:

- Fix duotone theme cache: WordPress/gutenberg#36236
- Fix duotone render in non-fse themes gutenberg: WordPress/gutenberg#37954
- Duotone: Allow users to specify custom filters gutenberg: WordPress/gutenberg#38442

Props oandregal, scruffian, Mamaduka.
Merges [52757] to the 5.9 branch.
See #55179.

Built from https://develop.svn.wordpress.org/branches/5.9@52759


git-svn-id: http://core.svn.wordpress.org/branches/5.9@52348 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this pull request Feb 17, 2022
This changeset is a backport for the following Gutenberg PRs:

- Fix duotone theme cache: WordPress/gutenberg#36236
- Fix duotone render in non-fse themes gutenberg: WordPress/gutenberg#37954
- Duotone: Allow users to specify custom filters gutenberg: WordPress/gutenberg#38442

Props oandregal, scruffian, Mamaduka.
Merges [52757] to the 5.9 branch.
See #55179.

Built from https://develop.svn.wordpress.org/branches/5.9@52759


git-svn-id: https://core.svn.wordpress.org/branches/5.9@52348 1a063a9b-81f0-0310-95a4-ce76da25c4cd
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this pull request Sep 18, 2022
This changeset is a backport for the following Gutenberg PRs:

- Fix duotone theme cache: WordPress/gutenberg#36236
- Fix duotone render in non-fse themes gutenberg: WordPress/gutenberg#37954
- Duotone: Allow users to specify custom filters gutenberg: WordPress/gutenberg#38442

Props oandregal, scruffian, Mamaduka.
Merges [52757] to the 5.9 branch.
See #55179.

Built from https://develop.svn.wordpress.org/branches/5.9@52759
VenusPR added a commit to VenusPR/Wordpress_Richard that referenced this pull request Mar 9, 2023
This changeset is a backport for the following Gutenberg PRs:

- Fix duotone theme cache: WordPress/gutenberg#36236
- Fix duotone render in non-fse themes gutenberg: WordPress/gutenberg#37954
- Duotone: Allow users to specify custom filters gutenberg: WordPress/gutenberg#38442

Props oandregal, scruffian, Mamaduka.
Merges [52757] to the 5.9 branch.
See #55179.

Built from https://develop.svn.wordpress.org/branches/5.9@52759


git-svn-id: http://core.svn.wordpress.org/branches/5.9@52348 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block Browser Issues Issues or PRs that are related to browser specific problems [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duotone filter does not work on front end for non FSE themes as of 12.1.0
5 participants