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

Undefined array key warning (image block) #55214

Closed
kafleg opened this issue Oct 10, 2023 · 15 comments · Fixed by #55269
Closed

Undefined array key warning (image block) #55214

kafleg opened this issue Oct 10, 2023 · 15 comments · Fixed by #55269
Labels
[Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended

Comments

@kafleg
Copy link
Member

kafleg commented Oct 10, 2023

Description

While testing WordPress 6.4 Beta 3, I found this warning.

Warning: Undefined array key 0 in C:\laragon\www\paidreviews\wp-includes\blocks\image.php on line 211

Step-by-step reproduction instructions

More info here, https://wordpress.slack.com/archives/C02RQBWTW/p1696955157600949

Screenshots, screen recording, code snippet

No response

Environment info

PHP: 8.1
WordPress 6.4 Beta 3 (Upgrade from beta 2 via CLI)
Laragon Local server

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@mikachan
Copy link
Member

I think this is likely related to this PR: #55010

Pinging @afercia @artemiomorales @dmsnell as folks who helped out with that PR. It looks like we should possibly check $img has items before using it.

@mikachan mikachan added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block labels Oct 10, 2023
@mikachan mikachan moved this to Needs Dev / Todo in WordPress 6.4 Editor Tasks Oct 10, 2023
@dmsnell
Copy link
Member

dmsnell commented Oct 10, 2023

This is failing here:

// Wrap the image in the body content with a button.
$img = null;
preg_match( '/<img[^>]+>/', $body_content, $img );

$button =
	$img[0]
	. '<button
		type="button"
		aria-haspopup="dialog"
		aria-label="' . esc_attr( $aria_label ) . '"
		data-wp-on--click="actions.core.image.showLightbox"
		data-wp-style--width="context.core.image.imageButtonWidth"
		data-wp-style--height="context.core.image.imageButtonHeight"
		data-wp-style--left="context.core.image.imageButtonLeft"
		data-wp-style--top="context.core.image.imageButtonTop"
	></button>';

in this section of code there are several places we assume that an image exists but don't check. I'm not sure why an image block has no IMG element, but it shouldn't crash WordPress when this happens.

@dmsnell
Copy link
Member

dmsnell commented Oct 10, 2023

@kafleg I followed the link for your reproduction steps but I didn't find any reproduction steps. can you share here? it would be helpful to know if this crash required manually breaking an image tag or if you took a normative sequence of actions that led to it. given that it seems like an image block needs to have no image in order for this to break, it's a bit surprising to me that it came up.

@kafleg
Copy link
Member Author

kafleg commented Oct 11, 2023

@dmsnell Once you enable the Expand on Click option in the image, the error will occur.
image

If you are still not able to produce it, I can make a short video.

@dmsnell
Copy link
Member

dmsnell commented Oct 11, 2023

A video would help @kafleg as I'm still unable to reproduce. It would also be helpful if you could post the text content of the post, the version displayed in the "Code Editor," reachable in the triple-dot "options" menu to the right of the sidebar buttons.

@RazzSunuwar
Copy link

I also found the same issue while enabling the new feature "Exapand on click". Before editing on editor, there is no issue but after making changes from the editor, the issues arises.

  • Manually installed WordPress 6.4 Beta 3
  • Set WP_debug to true on wp_config.php file
  • WordPress Dashboard>Appearance > Editor
  • Select the first image and enabled "Expand on click"
  • Save it and viewed on frontend.

Issue generated:
image

  • WordPress 6.4 Beta 3
  • PHP 7.4
  • Google Chrome Brower
  • Windwos 11
  • Laragon Localhost
  • TT4 Theme

@laxmanfalcha
Copy link

Same issue with me. I installed the latest version of WordPress 6.4 Beta 3. Set wp_debug in wp_config.php to true.

Before enabling the "Click on expand"
image

WordPress Dashboard>Appearance > Editor
After enabling the "Click on expand"
image

  • WordPress 6.4 Beta 3
  • PHP version: 8.1.10
  • Brower : Google Chrome
  • OS: Windows 11
  • Server: Laragon Local host
  • Theme: TT4 Theme

@artemiomorales artemiomorales moved this to Discovery in Image Lightbox Oct 11, 2023
@artemiomorales artemiomorales moved this from Discovery to Development Backlog in Image Lightbox Oct 11, 2023
@afercia
Copy link
Contributor

afercia commented Oct 11, 2023

I think this is likely related to this PR: #55010

Just to note that the regular expression and the assumption that an img tag is always found by not checking explicitly for $img[0] comes from code that dates before #55010. In #55010 we only changed the order of the markup.

@afercia
Copy link
Contributor

afercia commented Oct 11, 2023

I was able to find at least one way to reproduce this. It's similar to #55120 but it needs a specific fix.

It's similar because, basically, part of the image block code runs for every Image block on a page. The code assumes to find an img element within the block content. However, it appears this is not the case when an Image block is left with its block placeholder empty. Follow these steps:

  • Create a post.
  • Add a first image block and set its image to open in the lightbox.
  • Add a second image block but do not add an image, just keep the block placeholder empty.
  • Publish.
  • View the front end.
  • See the PHP notice Undefined offset: 0.
  • View the admin again and refresh the page.
  • See the PHP notice printed out also in the admin (although partially visually hidden by the editor UI).

A check for the 0 index appears to be the simplest fix.

However, I'm wondering why the block render callback runs even when the block is empty and actually 'contains' only the block placeholder in the first place.

@afercia
Copy link
Contributor

afercia commented Oct 11, 2023

As this can be reproduced in core, it needs to be fixed for the incoming WordPress release.
Cc @annezazu @bph @mikachan @SiobhyB @karmatosed @hellofromtonya

@dmsnell
Copy link
Member

dmsnell commented Oct 11, 2023

aha, thank you @afercia @RazzSunuwar and @laxmanfalcha - the missing piece for me was adding an image block without a placeholder. obviously we can always be defensive and check for existence, but I did want to understand how we got into this situation and now it makes sense that it's in pairing with another bug.

@artemiomorales
Copy link
Contributor

It's similar because, basically, part of the image block code runs for every Image block on a page. The code assumes to find an img element within the block content. However, it appears this is not the case when an Image block is left with its block placeholder empty.

@afercia Perfect, thanks for narrowing this down and finding the reproduction steps. I'll start working on a fix for this.

@dmsnell
Copy link
Member

dmsnell commented Oct 11, 2023

In WordPress/wordpress-develop#5461 and Core-59597 I've proposed a fix by checking for the presence of an IMG tag before processing the block. In WordPress/wordpress-develop#5428 I've proposed larger changes to the Lightbox rendering code, and that already incorporates this fix, but for the sake of the release it seems prudent to add the check and bail with the given input if no IMG can be found.

@artemiomorales
Copy link
Contributor

artemiomorales commented Oct 11, 2023

I've created a PR on the Gutenberg side in #55269

I'd prefer to do this on the Core side, but @gziolo informed me that the Core build process would strip out the changes.

Any eyes and approval on this would be super helpful! Thank you 🙏

@mikachan
Copy link
Member

I'd prefer to do this on the Core side, but @gziolo informed me that the Core build process would strip out the changes.

That's right, we'll need a fix on the Gutenberg side as the changes will be stripped when we update the packages in Core.

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 [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Status: Done
7 participants