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

Image Block: explicitly check for false from strpos() #38505

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Feb 3, 2022

Description

Following on from the conversation in #35975 (review)

This PR explicitly checks for a return value of false from strpos() just in case the check returns 0, which would also be falsey and satisfy the if-statement's condition.

If data-id were at the leading position in the string though, we'd have other problems :)

Props to @mcsf for catching it.

Testing Instructions

Insert a gallery and a single image into a post. Publish the page and head over to your site.

Make sure that the published gallery images include the data-id attribute on the <img /> tags. The single image will not have the data-id attribute.

Screenshots

Screen Shot 2022-02-04 at 9 40 35 am

Types of changes

Code qual.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

…content, which would satisfy the if-statement's condition.
@ramonjd ramonjd added [Type] Code Quality Issues or PRs that relate to code quality [Block] Image Affects the Image Block labels Feb 3, 2022
@ramonjd ramonjd requested review from mcsf and glendaviesnz February 3, 2022 22:53
@ramonjd ramonjd requested a review from ajitbohra as a code owner February 3, 2022 22:53
@ramonjd ramonjd self-assigned this Feb 3, 2022
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

LGTM 👍

✅ Gallery images get the data-id attribute
✅ Single images do not get the data-id attribute

@ramonjd ramonjd merged commit 037c82b into trunk Feb 4, 2022
@ramonjd ramonjd deleted the update/image-block-attribute-strpos-check branch February 4, 2022 02:26
@github-actions github-actions bot added this to the Gutenberg 12.6 milestone Feb 4, 2022
@mcsf
Copy link
Contributor

mcsf commented Feb 4, 2022

Thanks!

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] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants