Skip to content
Merged
Changes from 1 commit
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
Next Next commit
Fix PHP notice when an image with lightbox is deleted
  • Loading branch information
kishanjasani committed Oct 15, 2023
commit 439dc7e0edf12c14e06c670587c9910c8651e44d
10 changes: 6 additions & 4 deletions packages/block-library/src/image/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,17 @@ function block_core_image_render_lightbox( $block_content, $block ) {

// Note: We want to store the `src` in the context so we
// can set it dynamically when the lightbox is opened.
$img_width = 'none';
$img_height = 'none';
if ( isset( $block['attrs']['id'] ) ) {
$img_uploaded_src = wp_get_attachment_url( $block['attrs']['id'] );
$img_metadata = wp_get_attachment_metadata( $block['attrs']['id'] );
$img_width = $img_metadata['width'];
$img_height = $img_metadata['height'];
if ( ! empty( $img_metadata ) && is_array( $img_metadata ) ) {
$img_width = $img_metadata['width'];
$img_height = $img_metadata['height'];
}
Copy link
Member

@ramonjd ramonjd Oct 16, 2023

Choose a reason for hiding this comment

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

This could cause an error. Since img_width and img_height won't be set, $img_metadata won't pass the conditions.

Undefined variable $img_width in /var/www/html/wp-content/plugins/gutenberg/build/block-library/blocks/image.php on line 216 Warning: Undefined variable $img_height in /var/www/html/wp-content/plugins/gutenberg/build/block-library/blocks/image.php on line 217

Maybe we could provide a default value like the below else statement.

Suggested change
if ( ! empty( $img_metadata ) && is_array( $img_metadata ) ) {
$img_width = $img_metadata['width'];
$img_height = $img_metadata['height'];
}
$img_width = $img_metadata['width'] ?? 'none';
$img_height = $img_metadata['height'] ?? 'none';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @ramonjd, Thanks for the feedback. I already moved $img_width = 'none'; and $img_height = 'none'; out side of if statement. So, It won't throw error. I already checked the condition you mentioned with updated code.

Let me know, If I still need to make changes.

Copy link
Contributor Author

@kishanjasani kishanjasani Oct 16, 2023

Choose a reason for hiding this comment

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

By the way, Liked your short and optimal solution. I have updated the PR.

Thanks @ramonjd

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for working on this!

Looks good to me, but maybe @artemiomorales should run an eye past it just to be sure.

} else {
$img_uploaded_src = $processor->get_attribute( 'src' );
$img_width = 'none';
$img_height = 'none';
}

if ( isset( $block['attrs']['scale'] ) ) {
Expand Down