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.

Sign up for GitHub

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

Jump to bottom

Image: Stop crashing with Lightbox on image blocks without an image #55269

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Image: Stop crashing with Lightbox on image blocks without an image #55269

merged 2 commits into from
Oct 11, 2023

Conversation

artemiomorales
Copy link
Contributor

@artemiomorales artemiomorales commented Oct 11, 2023 •
edited
Loading

What?

Resolves #55214 #55217
Trac ticket: Core-59597

When rendering images with the lightbox "expand on click" function enabled, if an image block is encountered with no image (for example, a block was added but no image was ever selected), then WordPress would create a warning that an "undefined array key 0" was being accessed because the server code assumes an IMG element exists in the block's HTML.

In this patch a check is performed to ensure that an IMG block exists before processing the block. If one isn't, for any reason, the original given HTML for the block is passed through un-modified.

Testing Instructions

  1. Add an image block to a post, but don't add an image to it
  2. Publish and view the post
  3. See that there is no error in the content

@artemiomorales artemiomorales marked this pull request as ready for review October 11, 2023 15:17
@artemiomorales artemiomorales added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Feature] Interactivity API API to add frontend interactivity to blocks. labels Oct 11, 2023
@artemiomorales artemiomorales mentioned this pull request Oct 11, 2023
Closed
michalczaplinski
michalczaplinski approved these changes Oct 11, 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.

This tests well so I'm happy to approve 👍

One thing I don't understand 100%: Do we need to check both false === stripos( $content, '<img' AND ! $processor->next_tag( 'img' ) ? I probably misunderstood something but they seem redundant?

@dmsnell
Copy link
Member

dmsnell commented Oct 11, 2023

@michalczaplinski in a sense it's redundant but I figured that there's no reason to jump start the additional processing inside that function if we know that it's not possible to find an IMG element inside the content. I didn't measure it, so I don't have any strong feelings about it - it'd be fine to remove, fine to leave in.

@artemiomorales artemiomorales merged commit 9087e3d into trunk Oct 11, 2023
53 of 58 checks passed
@artemiomorales artemiomorales deleted the fix/image-array-key-warning branch October 11, 2023 18:36
@github-actions github-actions bot added this to the Gutenberg 16.9 milestone Oct 11, 2023
@mikachan
Copy link
Member

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

mikachan pushed a commit that referenced this pull request Oct 12, 2023
@artemiomorales @mikachan
Image: Stop crashing with Lightbox on image blocks without an image (#…
789ee91 
…55269)

* Stop crashing with Lightbox on image blocks without an image

* Fix PHPCS error
@mikachan mikachan removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 12, 2023
gziolo added a commit that referenced this pull request Oct 12, 2023
@mikachan @ellatrix
@peterwilsoncc @jsnajdr @afercia @gziolo @tellthemachines @artemiomorales @richtabor @ockham @felixarntz
Cherry-picked commits for WordPress 6.4 RC1 (#55298)
8449d79 
* List: fix forward merging of nested list (#55121)

* Private APIs: Update consent string for unlocking access. (#55182)

Replace the consent string with `I know using unstable features means my theme or plugin will inevitably break in the next version of WordPress`.

* useBlockSettings: add missing useMemo dependencies (#55204)

* Remove the lightbox filter and view file when the lightbox setting is disabled. (#55120)

* Remove the filter and view file of the lightbox when the lightbox setting is disabled.

* Make sure to add filter and view file when needed.

* Fix block comment indentation.

* Patterns: Remove the version enforcement for npm in `engines` field (#55245)

* Patterns: Remove the version enforcement for npm in `engines`

* Regenerate the lock file

* Remove `@return void` from PHP function docs. (#55237)

# Conflicts:
#	packages/block-library/src/form/index.php

* Image: Disable lightbox editor UI for linked images (#55141)

* Disable lightbox UI and add help text for linked images

* Update help text

* Image: Stop crashing with Lightbox on image blocks without an image (#55269)

* Stop crashing with Lightbox on image blocks without an image

* Fix PHPCS error

* Update fullscreen icon (#55021)

* Template Part block: Fall back to current theme if no theme attribute is given. (#55217)

* Template Part block: Fall back to current theme if no theme attribute is given.
* Remove now unnecessary _inject_theme_attribute_in_template_part_block() call from pattern block.

---------

Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@google.com>

---------

Co-authored-by: Ella <4710635+ellatrix@users.noreply.github.com>
Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>
Co-authored-by: Jarda Snajdr <jsnajdr@gmail.com>
Co-authored-by: Andrea Fercia <a.fercia@gmail.com>
Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
Co-authored-by: tellthemachines <tellthemachines@users.noreply.github.com>
Co-authored-by: Artemio Morales <artemio.morales@a8c.com>
Co-authored-by: Rich Tabor <hi@richtabor.com>
Co-authored-by: Bernie Reiter <96308+ockham@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@google.com>
@mikachan mikachan mentioned this pull request Oct 12, 2023
Closed
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Oct 12, 2023
@hellofromtonya
Editor: Update npm packages ahead of 6.4 RC1.
871a496 
Updates the npm packages and code for:

* [WordPress/gutenberg#55121 List: fix forward merging of nested list]

* [WordPress/gutenberg#55182 Update consent string for using private APIs.]

* [WordPress/gutenberg#55204 useBlockSettings: add missing useMemo dependencies]

* [WordPress/gutenberg#55120 Remove the lightbox filter and view file when the lightbox setting is disabled.]

* [WordPress/gutenberg#55245 Patterns: Remove the version enforcement for npm in engines field]

* [WordPress/gutenberg#55237 Remove `@return void` from PHP function docs]

* [WordPress/gutenberg#55141 Image: Disable lightbox editor UI for linked images]

* [WordPress/gutenberg#55269 Image: Stop crashing with Lightbox on image blocks without an image]

* [WordPress/gutenberg#55021 Update fullscreen icon]

* [WordPress/gutenberg#55217 Template Part block: Fall back to current theme if no theme attribute is given.] This change is part of fix for a performance regression re-introduced by [56818].

References:
* [WordPress/gutenberg#55298 Gutenberg PR 55298] Cherry-pick commits

Follow-up to [56818], [56816].

Props ellatrix, peterwilsoncc, jsnajdr, afercia, gziolo, isabel_brison, artemiosans, richtabor, bernhard-reiter, flixos90, mikachan, spacedmonkey, hellofromTonya.
See #59583, #59411.

git-svn-id: https://develop.svn.wordpress.org/trunk@56849 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to WordPress/WordPress that referenced this pull request Oct 12, 2023
@hellofromtonya
Editor: Update npm packages ahead of 6.4 RC1.
5ac7625 
Updates the npm packages and code for:

* [WordPress/gutenberg#55121 List: fix forward merging of nested list]

* [WordPress/gutenberg#55182 Update consent string for using private APIs.]

* [WordPress/gutenberg#55204 useBlockSettings: add missing useMemo dependencies]

* [WordPress/gutenberg#55120 Remove the lightbox filter and view file when the lightbox setting is disabled.]

* [WordPress/gutenberg#55245 Patterns: Remove the version enforcement for npm in engines field]

* [WordPress/gutenberg#55237 Remove `@return void` from PHP function docs]

* [WordPress/gutenberg#55141 Image: Disable lightbox editor UI for linked images]

* [WordPress/gutenberg#55269 Image: Stop crashing with Lightbox on image blocks without an image]

* [WordPress/gutenberg#55021 Update fullscreen icon]

* [WordPress/gutenberg#55217 Template Part block: Fall back to current theme if no theme attribute is given.] This change is part of fix for a performance regression re-introduced by [56818].

References:
* [WordPress/gutenberg#55298 Gutenberg PR 55298] Cherry-pick commits

Follow-up to [56818], [56816].

Props ellatrix, peterwilsoncc, jsnajdr, afercia, gziolo, isabel_brison, artemiosans, richtabor, bernhard-reiter, flixos90, mikachan, spacedmonkey, hellofromTonya.
See #59583, #59411.
Built from https://develop.svn.wordpress.org/trunk@56849


git-svn-id: http://core.svn.wordpress.org/trunk@56361 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Oct 12, 2023
@hellofromtonya
Editor: Update npm packages ahead of 6.4 RC1.
58d8b09 
Updates the npm packages and code for:

* [WordPress/gutenberg#55121 List: fix forward merging of nested list]

* [WordPress/gutenberg#55182 Update consent string for using private APIs.]

* [WordPress/gutenberg#55204 useBlockSettings: add missing useMemo dependencies]

* [WordPress/gutenberg#55120 Remove the lightbox filter and view file when the lightbox setting is disabled.]

* [WordPress/gutenberg#55245 Patterns: Remove the version enforcement for npm in engines field]

* [WordPress/gutenberg#55237 Remove `@return void` from PHP function docs]

* [WordPress/gutenberg#55141 Image: Disable lightbox editor UI for linked images]

* [WordPress/gutenberg#55269 Image: Stop crashing with Lightbox on image blocks without an image]

* [WordPress/gutenberg#55021 Update fullscreen icon]

* [WordPress/gutenberg#55217 Template Part block: Fall back to current theme if no theme attribute is given.] This change is part of fix for a performance regression re-introduced by [56818].

References:
* [WordPress/gutenberg#55298 Gutenberg PR 55298] Cherry-pick commits

Follow-up to [56818], [56816].

Props ellatrix, peterwilsoncc, jsnajdr, afercia, gziolo, isabel_brison, artemiosans, richtabor, bernhard-reiter, flixos90, mikachan, spacedmonkey, hellofromTonya.
See #59583, #59411.
Built from https://develop.svn.wordpress.org/trunk@56849


git-svn-id: https://core.svn.wordpress.org/trunk@56361 1a063a9b-81f0-0310-95a4-ce76da25c4cd
aaronjorbin pushed a commit to luminuu/wordpress-develop that referenced this pull request Oct 17, 2023
@hellofromtonya @aaronjorbin
Editor: Update npm packages ahead of 6.4 RC1.
183217c 
Updates the npm packages and code for:

* [WordPress/gutenberg#55121 List: fix forward merging of nested list]

* [WordPress/gutenberg#55182 Update consent string for using private APIs.]

* [WordPress/gutenberg#55204 useBlockSettings: add missing useMemo dependencies]

* [WordPress/gutenberg#55120 Remove the lightbox filter and view file when the lightbox setting is disabled.]

* [WordPress/gutenberg#55245 Patterns: Remove the version enforcement for npm in engines field]

* [WordPress/gutenberg#55237 Remove `@return void` from PHP function docs]

* [WordPress/gutenberg#55141 Image: Disable lightbox editor UI for linked images]

* [WordPress/gutenberg#55269 Image: Stop crashing with Lightbox on image blocks without an image]

* [WordPress/gutenberg#55021 Update fullscreen icon]

* [WordPress/gutenberg#55217 Template Part block: Fall back to current theme if no theme attribute is given.] This change is part of fix for a performance regression re-introduced by [56818].

References:
* [WordPress/gutenberg#55298 Gutenberg PR 55298] Cherry-pick commits

Follow-up to [56818], [56816].

Props ellatrix, peterwilsoncc, jsnajdr, afercia, gziolo, isabel_brison, artemiosans, richtabor, bernhard-reiter, flixos90, mikachan, spacedmonkey, hellofromTonya.
See #59583, #59411.

git-svn-id: https://develop.svn.wordpress.org/trunk@56849 602fd350-edb4-49c9-b593-d223f7449a82
@t-hamano t-hamano mentioned this pull request Dec 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalczaplinski michalczaplinski michalczaplinski approved these changes

@gziolo gziolo Awaiting requested review from gziolo

@dmsnell dmsnell Awaiting requested review from dmsnell

@afercia afercia Awaiting requested review from afercia

Assignees
No one assigned
Labels
[Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. [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.

Undefined array key warning (image block)
4 participants
@artemiomorales @dmsnell @mikachan @michalczaplinski