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

Shortcodes: Check if Brightcove shortcode attributes are an array before assuming they can be counted #8479

Merged
merged 1 commit into from
Jan 10, 2018
Merged

Shortcodes: Check if Brightcove shortcode attributes are an array before assuming they can be counted #8479

merged 1 commit into from
Jan 10, 2018

Conversation

oskosk
Copy link
Contributor

@oskosk oskosk commented Jan 6, 2018 •
edited
Loading

Fixes error being shown when used under PHP 7.2.

Part of #8156.

Changes proposed in this Pull Request:

  • Updates shortcodes/brightcove.php to check attributes with is_array() before assuming we can use count() on the argument passed to normalize_attributes().

Testing instructions:

  • Run phpunit --testsuite infinite-scroll under PHP 7.1, PHP 7.0 or PHP 5.6
  • Expect test to pass.

Proposed changelog entry for your changes:

  • Updated Brightcove shortcodes to be compatible with PHP 7.2.

@oskosk
Check if shortcode attributes are an array n Jetpack_Brightcove_Short…
024a322 
…code::normalize_attributes before assuming they can be counted
@oskosk oskosk added [Feature] Shortcodes / Embeds [Status] Needs Changelog [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Bug When a feature is broken and / or not performing as intended labels Jan 6, 2018
@oskosk oskosk requested a review from a team as a code owner January 6, 2018 21:10
@oskosk oskosk added this to the 5.8 milestone Jan 6, 2018
@jeherve jeherve mentioned this pull request Jan 6, 2018
Closed
13 tasks
@oskosk oskosk mentioned this pull request Jan 6, 2018
Merged
@oskosk oskosk changed the title Shortcodes: Check if Brightcode shortcode attributes are an array before assuming they can be counted Shortcodes: Check if Brightcove shortcode attributes are an array before assuming they can be counted Jan 8, 2018
dereksmart
dereksmart approved these changes Jan 10, 2018
View reviewed changes
Copy link
Member

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

LGTM

@jeherve jeherve 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 Jan 10, 2018
jeherve
jeherve approved these changes Jan 10, 2018
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Looks good. 🚢

@jeherve jeherve merged commit bf5bd83 into master Jan 10, 2018
@jeherve jeherve deleted the fix/count-error-brightcove-attributes-php72 branch January 10, 2018 19:15
jeherve added a commit that referenced this pull request Jan 29, 2018
@jeherve
Changelog: add #8478 and #8479
afd9245
zinigor pushed a commit that referenced this pull request Jan 30, 2018
@oskosk @zinigor
Changelog 5.8: running changelog (#8442)
1ee2c03 
* Changelog 5.8: create base for changelog.

* Update 5.8 release post link

* fix 5.8 release date

* Updates to plugin description

* Changelog: add #8499

* Changelog: add #8506

* Changelog: add #8509

* Changelog: add #8516

* Changelog: add #8517

* Changelog: add #8523

* Changelog: add #8547

* Changelog: add #8496

* Changelog: add #8584

* Changelog: add #8595

* Changelog: add #8445

* Changelog: add #8431

* Changelog: add #8284

* Changelog: add #8270

* Changelog: add #8124

* Changelog: add #8581

* Changelog: add #8463

* Changelog: add #8568 (#8646)

* Updates to testing list and changelog

* Changelog: add #8443

* Changelog: add #8459

* Changelog: add #8469

* Changelog: add #8464

* Changelog: add #8478 and #8479

* Changelog: add #8483

* Changelog: add #8488

* Changelog: add #8513

* Changelog: add #8555

* Changelog: add #8565

* Changelog: add #8601

* Changelog: add #8612

* Changelog: add first pass at Search items.

* Changelog: add more info to help test Search.

* Changelog: add #8144

* Changelog: add #8313

* Changelog: add #8419

* Changelog: add #8465

* Changelog: add #8515

* Changelog: add #8587

* Changelog: add #8591

* Changelog: add #8659

* Changelog: add #8661

* Changelog: add #8671

* Changelog: add 5.7.1 to archived changelog too.

* Reverted changes to readme, removed entry about backups.
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dereksmart dereksmart dereksmart approved these changes

@jeherve jeherve jeherve approved these changes

Assignees
No one assigned
Labels
[Feature] Shortcodes / Embeds Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
5.8
Development

Successfully merging this pull request may close these issues.
5 participants
@oskosk @jeherve @dereksmart @kraftbj @matticbot