Skip to content

Bug: PHP notice when an image with lightbox is deleted#55370

Merged
michalczaplinski merged 2 commits intoWordPress:trunkfrom
kishanjasani:fix/php-notice-on-image-deletation
Oct 16, 2023
Merged

Bug: PHP notice when an image with lightbox is deleted#55370
michalczaplinski merged 2 commits intoWordPress:trunkfrom
kishanjasani:fix/php-notice-on-image-deletation

Conversation

@kishanjasani
Copy link
Contributor

@kishanjasani kishanjasani commented Oct 15, 2023 •
edited
Loading

wp_get_attachment_metadata return false if it has no metadata related to attachment, So, I added a condition to check before extrtacting the value from array. it help fixes: #55347

Testing Instructions

  • Create a post or page.
  • Add an image from the Media Library and set it to Expand on Click: true.
  • Publish and view the front end.
  • Observe everything works as expected.
  • Go to the Media Library.
  • Delete the image that you previously added in the post.
  • View the previous post in the front end, refresh the page if necessary.
  • The image will be 'broken', and that is expected and PHP notice should not show.

@kishanjasani kishanjasani changed the title Fix PHP notice when an image with lightbox is deleted Fix: PHP notice when an image with lightbox is deleted Oct 15, 2023
@kishanjasani kishanjasani changed the title Fix: PHP notice when an image with lightbox is deleted Bug: PHP notice when an image with lightbox is deleted Oct 15, 2023
ramonjd
ramonjd reviewed Oct 16, 2023
View reviewed changes
packages/block-library/src/image/index.php Outdated
Comment on lines +175 to +178
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 •
edited
Loading

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

@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.

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 •
edited
Loading

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

@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.

Thank you for working on this!

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

@ramonjd ramonjd added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block labels Oct 16, 2023
michalczaplinski
michalczaplinski approved these changes Oct 16, 2023
View reviewed changes
Copy link
Contributor

@michalczaplinski michalczaplinski left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Thank you @kishanjasani

@michalczaplinski michalczaplinski merged commit d9a75d0 into WordPress:trunk Oct 16, 2023
@github-actions github-actions bot added this to the Gutenberg 16.9 milestone Oct 16, 2023
@afercia
Copy link
Contributor

afercia commented Oct 16, 2023

@mikachan I think the same change should be done for the lib/block-supports/behaviors.php file, just in case. Quick PR incoming.

@afercia afercia mentioned this pull request Oct 16, 2023
Merged
SiobhyB pushed a commit that referenced this pull request Oct 16, 2023
@kishanjasani @SiobhyB
Bug: PHP notice when an image with lightbox is deleted (#55370)
ab7369e 
* Fix PHP notice when an image with lightbox is deleted

* Fix PHP notice when an image with lightbox is deleted
@SiobhyB
Copy link
Contributor

SiobhyB commented Oct 16, 2023

I just cherry-picked this PR to the 6.4-rc1-2 branch to get it included in the next release: ab7369e

SiobhyB pushed a commit that referenced this pull request Oct 16, 2023
@mikachan @tellthemachines
@t-hamano @ciampo @spacedmonkey @andrewserong @kishanjasani
Cherry-picked commits for WordPress 6.4 RC1 v2 (#55348)
8e94be2 
* Add selector as id to layout style overrides. (#55291)

* Fix flickering when focusing on global style variations (#55267)

* ProgressBar: use text color to ensure enough contrast against background (#55285)

* Use text color at different opacities for track and indicator

* Add high contrast mode styles

* CHANGELOG
# Conflicts:
#	packages/components/CHANGELOG.md

* Remove empty attrs. (#54496)

* Remove empty attrs.

* Fix linter errors

---------

Co-authored-by: Sarah Norris <sarah@sekai.co.uk>

* Add IS_GUTENBERG_PLUGIN flag to LastRevision (#55253)

* useBlockPreview: Try outputting EditorStyles to ensure local style overrides are rendered (#55288)

* useBlockPreview: Try alternative fix for displaying local style overrides

* Avoid duplicate styles, fix rendering issues in Safari

* Add more explanatory comments

* Remove additional check for styles within the block preview, as it is not needed since EditorStyles handles its own style overrides retrieval

* Bug: PHP notice when an image with lightbox is deleted (#55370)

* Fix PHP notice when an image with lightbox is deleted

* Fix PHP notice when an image with lightbox is deleted

---------

Co-authored-by: tellthemachines <tellthemachines@users.noreply.github.com>
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Co-authored-by: Kishan Jasani <kishanjasani007@yahoo.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ramonjd ramonjd ramonjd left review comments

@michalczaplinski michalczaplinski michalczaplinski approved these changes

@artemiomorales artemiomorales Awaiting requested review from artemiomorales

Assignees

No one assigned

Labels

[Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

Gutenberg 16.9

Development

Successfully merging this pull request may close these issues.

Image block lightbox: PHP notices when an image with lightbox is deleted

6 participants

@kishanjasani @afercia @SiobhyB @michalczaplinski @ramonjd @mikachan