Skip to content

Use class_exists() guard for Featured_Content.#1

Merged
georgestephanis merged 1 commit intoAutomattic:2.7.1from
tierra:feature-content-guard
Dec 17, 2013
Merged

Use class_exists() guard for Featured_Content.#1
georgestephanis merged 1 commit intoAutomattic:2.7.1from
tierra:feature-content-guard

Conversation

@tierra
Copy link
Copy Markdown
Contributor

@tierra tierra commented Dec 16, 2013

This is a work-around to the problem exhibited by wp-cli/wp-cli#917, where the Twenty Fourteen theme manages to define the Featured_Content class first (as plugins are activated after the theme is loaded rather than before like core does).

This should ideally still be fixed in WP-CLI, but this workaround would still be helpful in the mean time.

@blobaugh
Copy link
Copy Markdown
Contributor

Putting the check around the class in modules/theme-tools/featured-content.php would probably be a better idea than around the include. That way it preserves the functionality in case the file gets included somewhere else.

If you can update it I will accept the pull request.

@georgestephanis
Copy link
Copy Markdown
Contributor

I actually think the way it is, more as it will preserve consistent merges for featured-content.php between wpcom and Jetpack when we run the merges between the source files there.

georgestephanis added a commit that referenced this pull request Dec 17, 2013
Use class_exists() guard for Featured_Content.

Necessary for wp-cli compatability, as wp-cli loads plugins -after- themes, as opposed to before like core does.

Implementing this in theme-tools.php rather than theme-tools/featured-content.php so as to maintain the file integrity of featured-content.php against its wpcom variant.

Props @tierra
@georgestephanis georgestephanis merged commit 643c84f into Automattic:2.7.1 Dec 17, 2013
@obenland
Copy link
Copy Markdown
Member

I'm not sure this is something we should do. Featured Content in Twenty Fourteen is meant to be pluggable, which is why it checks the $pagenow global for the plugins page to avoid that error.

If I'm reading Jetpack's code correctly, FC gets loaded on 'plugins_loaded', before any theme action anyway, no?

@tierra
Copy link
Copy Markdown
Contributor Author

tierra commented Dec 17, 2013

Normally, yes, but it wasn't in the case of WP-CLI (which wasn't setting $pagenow since it's not in the admin panel at all - it does now per wp-cli/wp-cli#917 though). The check doesn't change the loading behavior anywhere else, so it just helps not break in the rare case that it doesn't load in the order it normally does.

@obenland
Copy link
Copy Markdown
Member

it just helps not break in the rare case that it doesn't load in the order it normally does.

When is that ever the case though?

@tierra
Copy link
Copy Markdown
Contributor Author

tierra commented Dec 17, 2013

In the current version of WP-CLI, like I said.

@obenland
Copy link
Copy Markdown
Member

I was under the impression WP-CLI wasn't setting $pagenow, causing it to break, not changing the load order.

@tierra
Copy link
Copy Markdown
Contributor Author

tierra commented Dec 17, 2013

Twenty Fourteen uses the $pagenow check to avoid including it during plugin activation from the admin screen (where the order changes from a normal frontend page load) where it would break. When using WP-CLI though, because it never set $pagenow to plugins.php (because it's not actually a user using the admin screen, it shouldn't have to technically), Twenty Fourteen includes FC before WP-CLI activates Jetpack (in the same order the admin panel plugin activation does), where Jetpack was missing the guard, causing a fatal error.

Sorry if I gave the impression that the order is different when using WP-CLI, I was wrong about that initially. The load order does change, but it does it in both WP and in WP-CLI in the same way: it changes order only during plugin activation. This also still wouldn't normally be a problem because Jetpack only loads FC during the plugins_loaded action anyway, and that isn't triggered during plugin activation in wp-admin, but it apparently is in WP-CLI.

Putting all of this aside though, this is in fact a problem we've run into, and this change just makes Jetpack a little more robust, and less prone to failure. It doesn't actually change anything inside of normal use, so it really doesn't hurt. Should Twenty Fourteen have even included the FC class in a theme in the first place? Probably not, but now that it's included in two incredibly popular locations (a default theme, and the number one most installed plugin), it really isn't a bad idea to use an include guard just in case for all the weird and wild situations like WP-CLI in which Jetpack is used.

joshbetz added a commit that referenced this pull request Feb 8, 2014
samhotchkiss added a commit that referenced this pull request Jan 16, 2015
samhotchkiss added a commit that referenced this pull request Jan 22, 2015
gibrown pushed a commit to gibrown/jetpack that referenced this pull request Feb 26, 2015
Update color thief naming conventions.
samhotchkiss pushed a commit that referenced this pull request Apr 15, 2015
jeherve added a commit that referenced this pull request May 22, 2015
Related Posts: Make sure we escape the excerpt
ChrissiePollock added a commit that referenced this pull request Aug 12, 2015
Gallery functions: fix a few indentation issues
samhotchkiss pushed a commit that referenced this pull request Aug 14, 2015
jeherve pushed a commit that referenced this pull request Jul 18, 2016
After The Deadline: add spaces for WP coding standards.
gravityrail pushed a commit that referenced this pull request Jul 24, 2016
georgestephanis pushed a commit that referenced this pull request Feb 1, 2017
Form #wordpress.com-errors
> Fatal error for http://[private link].1/me/sites?http_envelope=1&site_visibility=all on web1.api.bur in /home/wpcom/public_html/wp-content/mu-plugins/post-flair/sharing/sharing-sources.php:371 - Uncaught TypeError: Argument 2 passed to Share_Twitter::__construct() must be of the type array, null given, called in /home/wpcom/public_html/wp-content/mu-plugins/post-flair/sharing

This would happen if option 'sharing options' was set, and was an array, but for some reason its 'global' key didn't exist or wasn't an array

It this case, it would also trigger a warning:
> Warning: array_merge(): Argument #1 is not an array in /home/wpcom/public_html/wp-content/mu-plugins/post-flair/sharing/sharing-service.php on line 222

Merges r146793-wpcom.
dhasilva pushed a commit that referenced this pull request Dec 20, 2024
dhasilva added a commit that referenced this pull request Dec 20, 2024
* Jetpack AI: Add thumbs up/down component to AI logo generator

* changelog

* Attemp #1 to fix some build errors

* changelog

* add base-styles to jetpack ai client

* move AiFeedbackThumbs to ai client

* avoid multiple events on same rating

* store rating with other logo information

* fix issue with persisting ratings with modal open

* add mediaLibraryId, prompt and revisedPrompt to event

---------

Co-authored-by: Douglas <douglas.henri@automattic.com>
jeherve added a commit that referenced this pull request May 7, 2025
This should avoid errors like:

Fatal error: Uncaught ValueError: NumberFormatter::__construct(): Argument #1 ($locale) "skr" is invalid

Internal reference: p1746629628168729-slack-CDLH4C1UZ

Some of the locale codes we pass to NumberFormatter are not always available. `skr` is a good example.
jeherve added a commit that referenced this pull request May 8, 2025
* Stats: avoid trying to access formatter for unavailable locale

This should avoid errors like:

Fatal error: Uncaught ValueError: NumberFormatter::__construct(): Argument #1 ($locale) "skr" is invalid

Internal reference: p1746629628168729-slack-CDLH4C1UZ

Some of the locale codes we pass to NumberFormatter are not always available. `skr` is a good example.

* Extract locale check to speed up locale detection

See #43396 (comment)

* Get locale with an underscore instead of dash

The dash is appropriate to display the language code in HTML, but is not valid ISO 639-3.

* Add test for new method

* Avoid static analysis error
chrisbliss18 added a commit that referenced this pull request Jul 8, 2025
Instagram responses with a value of "null" resulted in the following error:
PHP Fatal error: Uncaught TypeError: property_exists(): Argument #1 ($object_or_class) must be of
type object|string, null given in extensions/blocks/instagram-gallery/instagram-gallery.php:71

This change removes the error when a null response is received.
@edanzer edanzer mentioned this pull request Jul 22, 2025
3 tasks
jeherve added a commit that referenced this pull request Oct 6, 2025
Fixes #45386

This should avoid errors like this one:

```
An error of type E_ERROR was caused in line 298 of the file wp-content/plugins/jetpack/modules/photon-cdn.php. Error message: Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given in wp-content/plugins/jetpack/modules/photon-cdn.php:298
```

Co-authored-by: toothybrando@users.noreply.github.com
jeherve added a commit that referenced this pull request Oct 8, 2025
Fixes #45386

This should avoid errors like this one:

```
An error of type E_ERROR was caused in line 298 of the file wp-content/plugins/jetpack/modules/photon-cdn.php. Error message: Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given in wp-content/plugins/jetpack/modules/photon-cdn.php:298
```

Co-authored-by: toothybrando@users.noreply.github.com
gmjuhasz pushed a commit that referenced this pull request Oct 10, 2025
Fixes #45386

This should avoid errors like this one:

```
An error of type E_ERROR was caused in line 298 of the file wp-content/plugins/jetpack/modules/photon-cdn.php. Error message: Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given in wp-content/plugins/jetpack/modules/photon-cdn.php:298
```

Co-authored-by: toothybrando@users.noreply.github.com
jeherve added a commit that referenced this pull request Dec 12, 2025
Fixes JETPACK-1008

When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue:
- https://core.trac.wordpress.org/ticket/57579
- #46214

```
Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in …/wp-admin/admin-header.php on line 41
```

Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages.

Co-authored-by: Weston Ruter <weston@xwp.co>
jeherve added a commit that referenced this pull request Dec 16, 2025
* Jetpack settings: override module list & debugger page titles

Fixes JETPACK-1008

When registering the Module list page and the Jetpack Debugger page, we do not want the page to appear in the admin menu. This causes us to run into this issue:
- https://core.trac.wordpress.org/ticket/57579
- #46214

```
Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in …/wp-admin/admin-header.php on line 41
```

Let's override the title when we're on those pages to avoid the deprecation warning and have a proper page title on those 2 pages.

Co-authored-by: Weston Ruter <weston@xwp.co>

* changelog

* Change hook registration to match static declaration

See #46283 (comment)

---------

Co-authored-by: Weston Ruter <weston@xwp.co>
@claude claude bot mentioned this pull request Feb 4, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants