Make sure that the ogv property exists before using it#6527
Conversation
zinigor
left a comment
There was a problem hiding this comment.
One small nitpick, after that it's 🚢
| if ( isset( $this->video->videos->ogv ) ) { | ||
| $ogg = $this->video->videos->ogv->url; | ||
| if ( ! empty( $ogg ) ) { | ||
| $html .= '<source src="' . esc_url($ogg) . '" type="video/ogg; codecs="' . esc_attr($this->video->videos->ogv->codecs) . '"" />'; |
There was a problem hiding this comment.
Sorry for the nitpick, but you have removed spaces in braces here.
There was a problem hiding this comment.
PHPStorm removed them when I tabbed the block in, not sure why.
briancolinger
left a comment
There was a problem hiding this comment.
After checking that $this->video->videos->ogv->url is set you could just pass that to esc_url() and remove the need to set a new variable and then unset it.
Just nitpicking, but the esc_url() and esc_attr() calls need to have spacing on either side of the input variable.
|
@briancolinger: I'm not going to debate code style here, but this file was previously written by an old Automattician and the purpose of this pull request is to fix a notice. As of now, beyond this fix, this code is stable. I'm not going to potentially introduce new notices in the code by hastily updating the style of all of the HTML blocks in this code. If you would like to see this change happen, feel free to fix it in a new pull request. There are tons of places in the older VideoPress codebase that have style choices that look like this that can be fixed. :) |
|
Thanks for fixing the whitespace! I agree, fixing all the other things can wait until the next PR. |
* Readme: remove old release and add skeleton for 4.8. * Changelog: add #6572 * Changelog: add #6567 * Changelog: add #6542 * Changelog: add #6527 * Changelog: add #6508 * Changelog: add #6478 * Changelog: add #6477 * Changelog: add #6249 * Update stable version and remove old version from readme. * Changelog: add 4.7.1 to changelog. * Readme: add new contributor. * Sync: update docblock @SInCE version. Related: #6053 * Changelog: add release post. * changelog: add #6053 * Changelog: add #6413 * Changelog: add #6482 * Changelog: add #6584 * Changelog add #6603 * Changelog: add #6606 * Changelog: add #6611 * Changelog: add #6635 * Changelog: add #6639 * Changelog: add #6684 * Changelog: add #6710 * Changelog: add #6711 * Changelog: add #5461 * Testing list: update Settings UI feedback prompt. Props @MichaelArestad * Changelog: add #6789 * Changelog: add #6778 * Changelog: add #6777 * Changelog: add #6775 * Changelog: add #6755 * Changelog: add #6731 * Changelog: add #6721 * Changelog: add #6705 * Changelog: add #6702 * Changelog: add #6671 * Changelog: add #6637 * Changelog: add #6582 * Changelog: add #6566 * Changelog: add #6555 * Changelog: add #6529 * Changelog: add #6344 * Changelog: add #5763 * Changelog: add #5503 * Changelog: update #6637 changelog. @see 40e115c#commitcomment-21523982 * Changelog: add #6699 * Changelog: add #6632 * Changelog: add #6769 * Changelog: add #6707 * Changelog: add #6590
Fixes #6524
Note: stdClass sucks.