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

Add more data about the image as block props in the image block #11973

Closed
wants to merge 45 commits into from
Closed
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
4eb7ee5
Add fileWidth, fileHeight, userSet, editWidth props
azaozz Nov 16, 2018
fe99150
Merge branch 'master' into fix/image-block-add-props
azaozz Nov 16, 2018
1105451
Fix upscaling
azaozz Nov 16, 2018
1c5b8a0
Fix param/prop names
azaozz Nov 16, 2018
dc8133c
Merge branch 'master' into fix/image-block-add-props
azaozz Nov 17, 2018
03444d7
Better filter name, allow valueless attributes
azaozz Nov 17, 2018
fc7c8a6
Merge branch 'master' into fix/image-block-add-props
aduth Nov 20, 2018
1aa5b35
Compat: Resolve PHP lint errors
aduth Nov 20, 2018
2a9fba9
Compat: Correct closing img tag
aduth Nov 20, 2018
04706dd
Compat: Allow optional space after img tag name
aduth Nov 20, 2018
08473d8
Compat: PHPDoc formatting and arrangement style
aduth Nov 20, 2018
32d5c8c
Blocks: Use Math.round for image rounding
aduth Nov 20, 2018
cf50aec
Blocks: Add deprecation for attribute-sourced URL, alt
aduth Nov 20, 2018
e4cfea3
Blocks: Use hard-coded content width for block width
aduth Nov 20, 2018
4c49a8d
Blocks: Limit editWith assignment to insert, resize
aduth Nov 20, 2018
f9da97f
Blocks: Rename Image userSet to userSetDimensions
aduth Nov 20, 2018
8cef6ac
Blocks: Remove redundant userSetDimensions normalization
aduth Nov 20, 2018
84d6866
Blocks: Document constrainImageDimensions as ported from PHP
aduth Nov 20, 2018
db48c26
Blocks: Avoid mutative, unused Array#map result
aduth Nov 20, 2018
b2bc0e7
Block: Avoid overloading Image block updateImageURL
aduth Nov 20, 2018
f69ec35
Blocks: Fix Image accidental inline tab character
aduth Nov 20, 2018
009eceb
Compat: Avoid assumption of ID as non-last attribute of image
aduth Nov 20, 2018
bf9d58c
Compat: Document image block RegEx as temporary solution
aduth Nov 20, 2018
51e6c96
Blocks: Pick url, alt in raw transform from image node
aduth Nov 20, 2018
174bee0
Framework: Regenerate fixtures
aduth Nov 20, 2018
c26abe2
Blocks: Update image fixtures per url, alt as comment attributes
aduth Nov 20, 2018
579cf12
Compat: Align PHPDoc return
aduth Nov 20, 2018
9563082
Compat: Add PHPDoc since tag
aduth Nov 20, 2018
6efdb5c
Merge branch 'master' into fix/image-block-add-props
aduth Nov 20, 2018
a0a6dd9
Blocks: Remove outdated comment about floated images width accuracy
aduth Nov 20, 2018
8e73feb
Compat: Remove any attributes containing invalid characters
aduth Nov 20, 2018
8535d73
Blocks: Reintegrate media attributes pick to setAttributes spread
aduth Nov 20, 2018
4d2d959
Blocks: Set Image fileWidth, fileHeight only if both present
aduth Nov 20, 2018
19dad07
Compat: Use spec standard for valid attribute characters
aduth Nov 20, 2018
87130e4
Editor: Compute expected block width by DOM compute
aduth Nov 20, 2018
e2f8007
Compat: Always write img attribute value, even empty
aduth Nov 20, 2018
bf67408
Blocks: Avoid multiple calls to setAttributes in resetWidthHeight
aduth Nov 20, 2018
d8d7d6e
Update the 25-50-75-100% buttons
azaozz Nov 23, 2018
e3a99c7
Check only for valid HTML 5.0 attribute names
azaozz Nov 24, 2018
44c7c68
Update handling of "wide" and "full" alignment for the front-end
azaozz Nov 24, 2018
652ac92
Merge master
azaozz Nov 24, 2018
078a75b
Update `const getBlockWidth` to get the width from `div.editor-block-…
azaozz Nov 24, 2018
4be9bee
Fix typo in https://github.com/WordPress/gutenberg/pull/11973/commits…
azaozz Nov 25, 2018
4b0425f
Fix missing space to make wpcs happy
azaozz Nov 25, 2018
648811f
Update `gutenberg_render_block_core_image()`
azaozz Nov 28, 2018
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
2 changes: 1 addition & 1 deletion docs/language.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ _N.B.:_ The defining aspect of blocks are their semantics and the isolation mech
When blocks are saved to the content, after the editing session, its attributes—depending on the nature of the block—are serialized to these explicit comment delimiters.

```html
<!-- wp:image -->
<!-- wp:image {"url":"source.jpg"} -->
<figure class="wp-block-image">
<img src="source.jpg" alt="" />
</figure>
Expand Down
253 changes: 253 additions & 0 deletions lib/compat.php
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,259 @@ function gutenberg_warn_classic_about_blocks() {
<?php
}

/**
* Add 'actual_size' to the prepared attachment data for the Media Library.
*
* Needed as `wp_prepare_attachment_for_js()` (for the media modal) constrains
* the image sizes to the theme's `$content_width`.
*
* @param array $response Array of prepared attachment data.
* @param WP_Post $attachment Attachment object.
* @param array $meta Array of attachment meta data.
* @return array Array of prepared attachment data.
*/
function gutenberg_prepare_attachment_for_js( $response, $attachment, $meta ) {
if ( ! empty( $response['type'] ) && 'image' === $response['type'] && ! empty( $response['sizes'] ) ) {
foreach ( $response['sizes'] as $size_name => $size ) {
if ( array_key_exists( $size_name, $meta['sizes'] ) ) {
$response['sizes'][ $size_name ]['actual_size'] = array(
'width' => (int) $meta['sizes'][ $size_name ]['width'],
'height' => (int) $meta['sizes'][ $size_name ]['height'],
);
}
}
}

return $response;
}
add_filter( 'wp_prepare_attachment_for_js', 'gutenberg_prepare_attachment_for_js', 10, 3 );

/**
* Warm the object cache with post and meta information for all found image
* blocks to avoid making individual database calls (similarly to
* `wp_make_content_images_responsive()`).
*
* @access private
* @since 4.5.0
*
* @param string $content The post content.
* @return string $content Unchanged post content.
*/
function _gutenberg_cache_images_meta( $content ) {
// Need to find all image blocks and their attachment IDs BEFORE block
// filtering evaluates the rendered result so that attachements meta is
// retrieved all at once from the DB.
//
// [TODO]: When available, the regular expression should be avoided in
// favor of a filter on the parsed result of blocks, still prior to the
// rendered evaluation.
if ( preg_match_all( '/^<!-- wp:image {.*"id":(\d+).*} -->$/m', $content, $matches ) ) {
_prime_post_caches(
$matches[1],
/* $update_term_cache */ false,
/* $update_meta_cache */ true
);
}

return $content;
}
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason for this not to be hooking into the block filter so that we can get the actual attributes?

function _gutenberg_cache_images_meta( $html, $block ) {
	if ( 'core/image' === $block[ 'blockName' ] ) {
		_prime_post_caches( $block[ 'attrs' ][ 'id' ], /* why aren't these */ false, /* commented? */ true );
	}

	return $html;
}

add_filter( 'rendered_block', '_gutenberg_cache_images_meta', 3 );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose here is to get the IDs of all images (where we will do srcset and sizes) from the post before we parse the blocks. That's needed to get all image attachment meta in one go from the DB as that's faster.

So we need an array with all attachment IDs for the images used in the post. This is equivalent to https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/media.php#L1336.

That should also include images from "Cover" and "Media and Text" blocks, and probably the Gallery block (if images there are "hardcoded"). Any ideas how to do that without a regex are very welcome :)

Copy link
Member

Choose a reason for hiding this comment

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

Previously: #11377 (comment)

I'm not sure I understand why this would need to occur prior to the parse. If it at least occurs prior to any block evaluating its render_callback, wouldn't that be equally sufficient to warm the cache?

Copy link
Member

Choose a reason for hiding this comment

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

If it at least occurs prior to any block evaluating its render_callback, wouldn't that be equally sufficient to warm the cache?

Since this doesn't exist as an option today anyways (requires filter on this value), we probably can't do anything of this sort right now.

Should follow-up with a Trac ticket as appropriate.

Copy link
Contributor Author

@azaozz azaozz Nov 20, 2018

Choose a reason for hiding this comment

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

Yeah, this doesn't do anything if we do it "per block". It makes sense only when it happens for all of post_content at once before we start to look at individual images and generating srcset and sizes.

Copy link
Member

Choose a reason for hiding this comment

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

if we hook into the block-level filter (the design of which I favor in #10108) then we can track the parse through the blocks and then prime the cache on the do_blocks filter which occurs after parsing and rendering is complete.

that is, we could imagine something like this…

function _gutenberg_cache_images_meta( $html, $block ) {
	if ( $block['blockName'] === 'core/image' ) {
		$this->add_image( $block['attrs']['id'] );
	}

	return $html;
}
add_filter( 'rendered_block', '_gutenberg_cache_images_meta', 3 );

function _gutenberg_prime_cache() {
	if ( empty( $this->images ) ) {
		return;
	}

	// whatever command does all images in one go
	_prime_post_caches( $this->images, false, true, true, true, false, false );
}
add_filter( 'do_blocks', '_gutenberg_prime_cache' );

and this because we know that the final do_blocks filter runs after block-level processing.

Copy link
Member

Choose a reason for hiding this comment

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

noting that this is still based on a RegExp instead of using the per-block and per-document 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.

Another option would be to "unify" this with the "old" cache priming. We have the (required) wp-image-#### className on all images, including in Cover and Media and Text blocks.

Will need to abstract wp_make_content_images_responsive() a bit, move the caching out and run it earlier on the_content filter.

On the other hand _prime_post_caches() is "clever enough" to not fetch posts and meta that are already cached so running two pre-caching methods won't "hurt" anything.

Copy link
Member

Choose a reason for hiding this comment

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

I'm leaning towards doing this in conjunction with #10108 because I want to be sure the design is ergonomic enough to power this use case in a way that is scalable and transitive to other cases on other blocks.


// Run before blocks are parsed.
add_filter( 'the_content', '_gutenberg_cache_images_meta', 3 );

/**
* Calculates the image width and height based on $block_witdh and the
* `editWidth` block attribute.
*
* @since 4.5.0
*
* @param array $block_attributes The block attributes.
* @param array $image_meta Optional. The image attachment meta data.
* @return array|bool An array of the image width and height, in that order, or
* false if the image data is missing from $block_attributes.
*/
function gutenberg_get_image_width_height( $block_attributes, $image_meta = null ) {
if ( ! empty( $block_attributes['width'] ) && ! empty( $block_attributes['height'] ) ) {
// The image was resized.
$image_dimensions = array(
$block_attributes['width'],
$block_attributes['height'],
);

/*
* Here we can use `$block_attributes['editWidth']` to scale the image
* if we know the theme's "expected width" (in pixels).
*
* Note that if the `$block_attributes['userSetDimensions']` is set/true, the user has entered
* the width and height by hand, they shouldn't probably be changed.
*
* Something like:
* if ( empty( $block_attributes['userSetDimensions'] ) && ! empty( $block_attributes['editWidth'] ) && $content_width <> $block_attributes['editWidth'] ) {
* // Scale the image if the block width in the editor was different than the current theme width.
* $scale = $content_width / $block_attributes['editWidth'];
* $image_width = round( $block_attributes['width'] * $scale );
* }
* $image_dimensions = wp_constrain_dimensions( $image_file_width, $image_file_height, $image_width );
*/
} elseif ( ! empty( $block_attributes['fileWidth'] ) && ! empty( $block_attributes['fileHeight'] ) ) {
$image_dimensions = array(
$block_attributes['fileWidth'],
$block_attributes['fileHeight'],
);
} else {
return false;
}

/*
* Do not constrain images with "wide" and "full" alignment to the "large" image size.
* TODO: To reduce (fix) the need for upscaling or using the "full" size images
* add "xlarge" image size generated by default!
*/
if ( ! empty( $image_meta ) &&
( 'wide' === $block_attributes['align'] || 'full' === $block_attributes['align'] ) &&
! empty( $block_attributes['fileWidth'] ) &&
$block_attributes['fileWidth'] < $image_meta['width'] &&
max( (int) $image_meta['width'], (int) $image_meta['height'] ) < 4300 && // max 12 MP photo (that's still may be over 3MB, consider reducing).
Copy link
Member

Choose a reason for hiding this comment

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

This line gives me some pause because it introduces caveats to the claim that "Full wide alignment uses the original image", the number itself maybe (inherently) arbitrary, and very much a magic number (should be constant? filtered value?).

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 a pretty... arbitrary assumption. We just need a "sensible default" for now.

Really hoping we can add another larger image size that's generated by default in core, like xlarge. Then we can just use it or fall back to "full" when it doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this to look for larger image sizes that were added by themes or plugins first, see 44c7c68.

! empty( $image_meta['sizes']['large'] ) &&
$block_attributes['fileWidth'] === (int) $image_meta['sizes']['large']['width'] &&
Copy link
Member

Choose a reason for hiding this comment

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

I'm not clear why this condition exists if at least the next line assures an equal aspect ratio.

Copy link
Contributor Author

@azaozz azaozz Nov 20, 2018

Choose a reason for hiding this comment

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

It's an edge case check. Image attachment meta in core may be... inconsistent in rare cases. This is an "just in time" check to make sure we don't trigger PHP warnings (just nice to have).

wp_image_matches_ratio( $block_attributes['fileWidth'], $block_attributes['fileHeight'], $image_meta['width'], $image_meta['height'] )
) {
$image_dimensions = array(
$image_meta['width'],
$image_meta['height'],
);
}

/**
* Filters the image size for the image block.
*
* @since 4.5.0
*
* @param array $image_size The calculated image size width and
* height (in that order).
* @param array $block_attributes The block attributes.
* @param array $image_meta The image attachment meta data.
*/
return apply_filters( 'block_core_image_get_width_height', $image_dimensions, $block_attributes, $image_meta );
}

/**
* Filters the rendered output of the Image block to include generated HTML
* attributes for front-end display.
*
* @since 4.5.0
*
* @param string $html Original HTML.
* @param array $block Parsed block.
* @return string Filtered Image block HTML.
*/
function gutenberg_render_block_core_image( $html, $block ) {
// Old post or... something's wrong.
if ( empty( $html ) || empty( $block['attrs'] ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should be shortcutting here if the block is not the image block? It'll eventually be shortcutted at the below test of empty( $block_attributes['url'] ), but it seems both indirect and wasteful to let us get that far, when this is called for every block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good catch. Added in 648811f.

return $html;
}

$defaults = array(
'url' => '',
'alt' => '',
'id' => 0,
'align' => '',
);

$block_attributes = wp_parse_args( $block['attrs'], $defaults );

if ( empty( $block_attributes['url'] ) ) {
// We don't have enough data to construct new img tag. Fall back to the existing HTML.
return $html;
}

if ( ! empty( $block_attributes['id'] ) ) {
$attachment_id = (int) $block_attributes['id'];
$image_meta = wp_get_attachment_metadata( $attachment_id );
} else {
$attachment_id = 0;
$image_meta = null;
}

$image_dimensions = gutenberg_get_image_width_height( $block_attributes, $image_meta );
$image_src = '';
$srcset = '';
$sizes = '';

if ( empty( $image_dimensions ) ) {
// We don't have enough data to construct new img tag. Fall back to the existing HTML.
return $html;
}

$image_src = $block_attributes['url'];

$image_attributes = array(
'src' => $image_src,
'alt' => empty( $block_attributes['alt'] ) ? '' : $block_attributes['alt'],
'width' => $image_dimensions[0],
'height' => $image_dimensions[1],
);

if ( $image_meta ) {
// TODO: pass `$block_attributes` to the filter.
$srcset = wp_calculate_image_srcset( $image_dimensions, $image_src, $image_meta, $attachment_id );

if ( ! empty( $srcset ) ) {
// TODO: pass `$block_attributes` to the filter. This will let themes generate better `sizes` attribute.
$sizes = wp_calculate_image_sizes( $image_dimensions, $image_src, $image_meta, $attachment_id );
}

if ( $srcset && $sizes ) {
$image_attributes['srcset'] = $srcset;
$image_attributes['sizes'] = $sizes;
}
}

/**
* Filters the image tag attributes when rendering the core image block.
*
* @since 4.5.0
*
* @param array $image_attributes The (recalculated) image attributes.
* Note: expects valid HTML 5.0 attribute names.
* @param array $block_attributes The image block attributes.
* @param string $html The image block HTML coming from the
* editor. The img tag will be replaced.
*/
$image_attributes = apply_filters( 'render_block_core_image_tag_attributes', $image_attributes, $block_attributes, $html );

$attr = '';
foreach ( $image_attributes as $name => $value ) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not super confident in going this route (i.e. rebuilding the markup) because it carries a lot of overhead in future updates to the image block client side.

Also, I worry we package a lot of this functionality in the image block alone, and any other block that uses images, misses on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using block props to rebuild the <img> tags seems the best option. It would be trivial to add support for more/other attributes in the future.

Of course we can always go "the other route" like in the current processing when adding srcset and sizes: use regex to parse some of the attributes, then add or replace them with recalculated values.

// Sanitize for valid HTML 5.0 attribute names
//
// See: https://www.w3.org/TR/html52/syntax.html#attribute-names .
$is_invalid_attribute = preg_match( '/[\\\\u007F-\\\\u009F "\'>\/="\\\\uFDD0-\\\\uFDEF]/', strtolower( $name ) );

if ( $is_invalid_attribute ) {
continue;
}

if ( 'src' === $name ) {
$value = esc_url( $value );
} elseif ( ( 'width' === $name || 'height' === $name ) && ! empty( $value ) ) {
$value = (int) $value;
} else {
$value = esc_attr( $value );
}

$attr .= sprintf( ' %s="%s"', $name, $value );
}

$image_tag = '<img' . $attr . ' />';

// Replace the img tag.
$html = preg_replace( '/<img\s?[^>]+>/', $image_tag, $html );

return $html;
}
// Needs WP 5.0-beta4 to work.
add_filter( 'render_block', 'gutenberg_render_block_core_image', 10, 2 );

/**
* Display the privacy policy help notice.
*
Expand Down
Loading