Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions lib/block-supports/layout.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,36 @@ function( $matches ) {
remove_filter( 'render_block', 'wp_restore_group_inner_container', 10, 2 );
}
add_filter( 'render_block', 'gutenberg_restore_group_inner_container', 10, 2 );


/**
* For themes without theme.json file, make sure
* to restore the outer div for the aligned image block
* to avoid breaking styles relying on that div.
*
* @param string $block_content Rendered block content.
* @param array $block Block object.
* @return string Filtered block content.
*/
function gutenberg_restore_image_outer_container( $block_content, $block ) {
$image_with_align = '/(^\s*<figure\b[^>]*)wp-block-image\s([^"]*((alignleft)|(alignright)|(aligncenter))(\s|")[^>]*>((.|\S|\s)*)<\/figure>)/U';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ockham @mcsf if you have any thoughts about my regex (I'm not an expert there :P)

Copy link
Member

@Mamaduka Mamaduka Feb 9, 2022

Choose a reason for hiding this comment

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

nitpick (non-blocker): We can probably return earlier if the block isn't an image or doesn't have theme.json.

if (
  'core/image' !== $block['blockName'] ||
  WP_Theme_JSON_Resolver::theme_has_support()
) {
  return $block_content;
}

P.S. Sorry, I'm also not an expert here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's what it does already but since this is just an assignment, I think it's fine. (the variable is also used for the early return)

Copy link
Contributor

Choose a reason for hiding this comment

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

Notes from reviewing the regular expression:

  1. Word boundary needed before "wp-block-image", otherwise you would match <figure class="not-wp-block-image […]. Fix: \bwp-block-image.

  2. The RE doesn't target the class attribute specifically; shouldn't it? Right now I can match <figure not-class="....

  3. No parens needed around each string: (alignleft|alignright|aligncenter)

  4. Many of the remaining groups don't need capturing: we only care about $match[1] and $match[2], so the above example could become (?:alignleft|alignright:aligncenter), and same for (?:\s|").

  5. ((.|\S|\s)*) is amusing! \S|\s means "I want to match something as well as its opposite", i.e. "everything", which is the same as .. So, unless I missed something, the outer group could be .*

  6. (\s|"): I'm not sure what purpose this serves. Oh, maybe it's used as a word boundary after the alignment class? Right, and now I notice that there is missing word boundary at the start of the alignment class. You'll see that right now you're able to (incorrectly) match zzzzalignleft. So let's use proper word boundaries on both sides. Fix: \b(?:alignleft|alignright|aligncenter)\b.

  7. (I don't think I'd ever seen the U (ungreedy) flag in use, that's cool)

Based on all of the above except for the fact that we don't target the class attribute, the RE could be:

/(^\s*<figure\b[^>]*)\bwp-block-image\b([^"]*\b(?:alignleft|alignright|aligncenter)\b[^>]*>.*<\/figure>)/U

I've tested this in the venerable regexr.com, but not with live content, so take my review accordingly. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the first remark, it could also be "wp-block-image, is " a word boundary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2- It could target class to be 100% accurate but I didn't think it was mandatory (like group blocks).

5- ((.|\S|\s)) is amusing! \S|\s means "I want to match something as well as its opposite", i.e. "everything", which is the same as .. So, unless I missed something, the outer group could be .

Indeed, but for some reason in the site I was using . was not catching everything (I think related to multiline)

6- (\s|") this one targets a space or end of attribute (if it's the last class)

Copy link
Contributor

Choose a reason for hiding this comment

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

For the first remark, it could also be "wp-block-image, is " a word boundary?

That should work just as well, if you're sure that wp-block-image is always the first class.

Indeed, but for some reason in the site I was using . was not catching everything (I think related to multiline)

Hm, interesting. Take my advice with caution, then. :) But make sure the trick is really necessary.


if (
'core/image' !== $block['blockName'] ||
WP_Theme_JSON_Resolver::theme_has_support() ||
0 === preg_match( $image_with_align, $block_content )
) {
return $block_content;
}

$updated_content = preg_replace_callback(
$image_with_align,
static function( $matches ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the effect of the static keyword here? I thought static anonymous function declarations only mattered in the context of a class (docs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know it was copied from the "group" implementation but If I'm not wrong most of these static were introduced in a patch to support php 8 or something.

return '<div class="wp-block-image">' . $matches[1] . $matches[2] . '</div>';
},
$block_content
);
return $updated_content;
}

add_filter( 'render_block', 'gutenberg_restore_image_outer_container', 10, 2 );
Copy link
Member

Choose a reason for hiding this comment

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

Was there a reason for not using render_block_core/image as the filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No particular reason, I guess my reasoning is that this change is tied to the "layout support" so I put here similarly to the previous one we have for group blocks. (same use-case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe you mean why not use the render_block_core/image filter name? If that's the case, I just didn't know that these block specific filters exist.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, sorry for not being clear here. The filter was introduced in 5.7, see https://core.trac.wordpress.org/changeset/50123. It basically allows you to skip the 'core/image' !== $block['blockName'] check and only runs if the block is actually used on a page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice 👍 We should probably use it for both image and group filters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened a follow-up here #38657

106 changes: 105 additions & 1 deletion packages/block-library/src/image/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
* External dependencies
*/
import classnames from 'classnames';
import { isEmpty } from 'lodash';

/**
* WordPress dependencies
*/
import { RichText } from '@wordpress/block-editor';
import { RichText, useBlockProps } from '@wordpress/block-editor';

const blockAttributes = {
align: {
Expand Down Expand Up @@ -68,7 +69,110 @@ const blockAttributes = {
},
};

const blockSupports = {
anchor: true,
color: {
__experimentalDuotone: 'img',
text: false,
background: false,
},
__experimentalBorder: {
radius: true,
__experimentalDefaultControls: {
radius: true,
},
},
};

const deprecated = [
{
attributes: {
...blockAttributes,
title: {
type: 'string',
source: 'attribute',
selector: 'img',
attribute: 'title',
},
sizeSlug: {
type: 'string',
},
},
supports: blockSupports,
save( { attributes } ) {
const {
url,
alt,
caption,
align,
href,
rel,
linkClass,
width,
height,
id,
linkTarget,
sizeSlug,
title,
} = attributes;

const newRel = isEmpty( rel ) ? undefined : rel;

const classes = classnames( {
[ `align${ align }` ]: align,
[ `size-${ sizeSlug }` ]: sizeSlug,
'is-resized': width || height,
} );

const image = (
<img
src={ url }
alt={ alt }
className={ id ? `wp-image-${ id }` : null }
width={ width }
height={ height }
title={ title }
/>
);

const figure = (
<>
{ href ? (
<a
className={ linkClass }
href={ href }
target={ linkTarget }
rel={ newRel }
>
{ image }
</a>
) : (
image
) }
{ ! RichText.isEmpty( caption ) && (
<RichText.Content
tagName="figcaption"
value={ caption }
/>
) }
</>
);

if ( 'left' === align || 'right' === align || 'center' === align ) {
return (
<div { ...useBlockProps.save() }>
<figure className={ classes }>{ figure }</figure>
</div>
);
}

return (
<figure { ...useBlockProps.save( { className: classes } ) }>
{ figure }
</figure>
);
},
},
{
attributes: blockAttributes,
save( { attributes } ) {
Expand Down
8 changes: 0 additions & 8 deletions packages/block-library/src/image/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,6 @@ export default function save( { attributes } ) {
</>
);

if ( 'left' === align || 'right' === align || 'center' === align ) {
return (
<div { ...useBlockProps.save() }>
<figure className={ classes }>{ figure }</figure>
</div>
);
}

return (
<figure { ...useBlockProps.save( { className: classes } ) }>
{ figure }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:core/image {"align":"center"} -->
<div class="wp-block-image"><figure class="aligncenter"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure></div>
<figure class="wp-block-image aligncenter"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure>
<!-- /wp:core/image -->
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
"align": "center"
},
"innerBlocks": [],
"innerHTML": "\n<div class=\"wp-block-image\"><figure class=\"aligncenter\"><img src=\"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure></div>\n",
"innerHTML": "\n<figure class=\"wp-block-image aligncenter\"><img src=\"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure>\n",
"innerContent": [
"\n<div class=\"wp-block-image\"><figure class=\"aligncenter\"><img src=\"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure></div>\n"
"\n<figure class=\"wp-block-image aligncenter\"><img src=\"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure>\n"
]
}
]
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:image {"align":"center"} -->
<div class="wp-block-image"><figure class="aligncenter"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt=""/><figcaption>Give it a try. Press the "really wide" button on the image toolbar.</figcaption></figure></div>
<figure class="wp-block-image aligncenter"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt=""/><figcaption>Give it a try. Press the "really wide" button on the image toolbar.</figcaption></figure>
<!-- /wp:image -->
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:image {"align":"left"} -->
<div class="wp-block-image"><figure class="alignleft"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt=""/></figure></div>
<figure class="wp-block-image alignleft"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt=""/></figure>
<!-- /wp:image -->
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:image {"align":"left","width":100,"height":100} -->
<div class="wp-block-image"><figure class="alignleft is-resized"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" width="100" height="100"/></figure></div>
<figure class="wp-block-image alignleft is-resized"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" width="100" height="100"/></figure>
<!-- /wp:image -->
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:image {"align":"left","width":100,"height":100} -->
<div class="wp-block-image"><figure class="alignleft is-resized"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" width="100" height="100"/></figure></div>
<figure class="wp-block-image alignleft is-resized"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" width="100" height="100"/></figure>
<!-- /wp:image -->
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!-- wp:image {"align":"left","id":13,"width":358,"height":164,"sizeSlug":"large","linkDestination":"none","style":{"border":{"radius":"10px"}}} -->
<div class="wp-block-image" style="border-radius:10px"><figure class="alignleft size-large is-resized"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" class="wp-image-13" width="358" height="164"/></figure></div>
<!-- /wp:image -->
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[
{
"name": "core/image",
"isValid": true,
"attributes": {
"align": "left",
"url": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==",
"alt": "",
"caption": "",
"id": 13,
"width": 358,
"height": 164,
"linkDestination": "none",
"sizeSlug": "large",
"style": {
"border": {
"radius": "10px"
}
}
},
"innerBlocks": []
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[
{
"blockName": "core/image",
"attrs": {
"align": "left",
"id": 13,
"width": 358,
"height": 164,
"sizeSlug": "large",
"linkDestination": "none",
"style": {
"border": {
"radius": "10px"
}
}
},
"innerBlocks": [],
"innerHTML": "\n<div class=\"wp-block-image\" style=\"border-radius:10px\"><figure class=\"alignleft size-large is-resized\"><img src=\"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==\" alt=\"\" class=\"wp-image-13\" width=\"358\" height=\"164\"/></figure></div>\n",
"innerContent": [
"\n<div class=\"wp-block-image\" style=\"border-radius:10px\"><figure class=\"alignleft size-large is-resized\"><img src=\"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==\" alt=\"\" class=\"wp-image-13\" width=\"358\" height=\"164\"/></figure></div>\n"
]
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!-- wp:image {"align":"left","id":13,"width":358,"height":164,"sizeSlug":"large","linkDestination":"none","style":{"border":{"radius":"10px"}}} -->
<figure class="wp-block-image alignleft size-large is-resized" style="border-radius:10px"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="" class="wp-image-13" width="358" height="164"/></figure>
<!-- /wp:image -->
2 changes: 1 addition & 1 deletion test/integration/fixtures/documents/wordpress-out.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ <h3>Shortcode</h3>
<!-- /wp:image -->

<!-- wp:image {"align":"right","id":5114} -->
<div class="wp-block-image"><figure class="alignright"><img src="aligned.png" alt="" class="wp-image-5114"/></figure></div>
<figure class="wp-block-image alignright"><img src="aligned.png" alt="" class="wp-image-5114"/></figure>
<!-- /wp:image -->

<!-- wp:paragraph -->
Expand Down