-
Notifications
You must be signed in to change notification settings - Fork 808
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
Responsive Videos: avoid notice when param is not given. #10109
Conversation
Caution: This PR has changes that must be merged to WordPress.com |
That's a great PR description, thank you so much for your effort! Generated by 🚫 dangerJS |
I don't see any PHP Notices even without the patch. What am I doing wrong? Theme: 2017 |
It seems to only happen with some specific themes. The last user report was from a user using a Premium theme, so I could not download it to test either. :( I assume that some themes call the |
I can't reproduce the warning with the provided steps :( Found that a theme like |
It looks like that theme includes a full copy fo the Responsive Videos tool: |
I wasn't able to reproduce it either :( |
Just realized that this PR is introducing something that was already there. I'm merging it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM given that this piece of code was already supposed to be like this
* Readme: add boilerplate for next release, 6.6 * Add 6.5 to the changelog.txt file * Set boilerplate testing list for 6.6 * Readme: update stable tag to 6.5 * Add bullets to 6.5 changelog items * Readme: add link to previous changelogs This will help folks who want to know more about past releases, while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release. * Changelog: add information at the top of the changelog file. * Changelog: add #10054 * Changelog: add #10078 * Changelog: add #10079 * Changelog: add #10064 * Changelog: add #10094 * Changelog: add #10096 * Testing list: add more information based on #10087 * Changelog: add #9847 * Changelog: add #10084 * Changelog: add #9918 * Changelog: add #7614 * Changelog: add #10116 * Changelog: add #10108 * Changelog: add #10041 * Changelog: add #10121 * Changelog: add #10134 * Changelog: add #10130 * Changelog: add #10109 * changelog: add #10137 * changelog: add #9952 * changelog: add #10120 * changelog: add #10162 * Changelog: add #10163 * Changelog: add #10092 * changelog: add #10156 * Changelog: add #10154 * changelog: add #10122 * Changelog: add #10101 * changelog: add #10105 * changelog: add #10190 * Changelog: add #10196 * changelog: add #10152 * Changelog: add #10153 * Testing list: add more details to Site Verification testing steps. @see #10143 (comment) * changelog: add #10194 * Changelog: add #10193
Fixes #3048
Changes proposed in this Pull Request:
Avoid PHP notice when param is not given.
This was already fixed in #3243, but got reverted by mistake in ab7f2e0
Testing instructions:
Proposed changelog entry for your changes:
Responsive Videos: avoid PHP notice.