Skip to content

Fix PHP notice when image width/height not available in Jetpack_AMP_Support::add_image_to_metadata()#10031

Merged
abidhahmed merged 1 commit intoAutomattic:masterfrom
xwp:fix/amp-image-metadata-php-notice
Aug 20, 2018
Merged

Fix PHP notice when image width/height not available in Jetpack_AMP_Support::add_image_to_metadata()#10031
abidhahmed merged 1 commit intoAutomattic:masterfrom
xwp:fix/amp-image-metadata-php-notice

Conversation

@westonruter
Copy link
Copy Markdown
Contributor

@westonruter westonruter commented Aug 16, 2018 •
edited by gravityrail
Loading

Changes proposed in this Pull Request:

This fixes an issue I saw in my PHP error log:

Undefined index: src_width in wp-content/plugins/jetpack/3rd-party/class.jetpack-amp-support.php on line 224

Testing instructions:

The issue occurs on a post that has no featured image set, but which does have an embedded gallery shortcode, where the underlying image files have been deleted for the attachments in that gallery.

True this is an edge case, but this hardening seems it can't hurt.

Proposed changelog entry for your changes:

Fix PHP notice when rendering AMP images with unknown width and height.

@westonruter westonruter requested a review from a team as a code owner August 16, 2018 14:33
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Aug 16, 2018

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review
⚠️

"Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS

@abidhahmed abidhahmed added Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. AMP labels Aug 17, 2018
@abidhahmed abidhahmed requested a review from gravityrail August 17, 2018 07:20
gravityrail
gravityrail approved these changes Aug 17, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@gravityrail gravityrail left a comment

Choose a reason for hiding this comment

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

🎉

@gravityrail gravityrail added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 17, 2018
@gravityrail gravityrail added this to the 6.5 milestone Aug 17, 2018
@abidhahmed abidhahmed merged commit 8f02a62 into Automattic:master Aug 20, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 20, 2018
oskosk pushed a commit that referenced this pull request Aug 28, 2018
@abidhahmed @oskosk
[skip-ci] Added changelogs
7c86fd2 
#9958 
#9983 
#9992 
#10002 
#10004
#10010
#10011
#10017
#10031
#10037
#10039
#10040
#10051
oskosk pushed a commit that referenced this pull request Aug 28, 2018
@abidhahmed @oskosk
[skip-ci] Add test instructions
1942746 
#9958
#9983
#9992 
#10002
#10004
#10010
#10017
#10031
#10037
#10039
#10040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@gravityrail gravityrail gravityrail approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

AMP Bug When a feature is broken and / or not performing as intended

Projects

None yet

Milestone

6.5

Development

Successfully merging this pull request may close these issues.

5 participants

@westonruter @jetpackbot @gravityrail @abidhahmed @matticbot