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

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

Merged
merged 1 commit into from
Aug 20, 2018
Merged

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

merged 1 commit into from
Aug 20, 2018

Conversation

westonruter
Copy link
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

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 [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. 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
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 To request a review from fellow Jetpack developers. Label will be renamed soon. 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

@gravityrail gravityrail gravityrail approved these changes

Assignees
No one assigned
Labels
AMP [Type] 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