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.

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

Use class_exists() guard for Featured_Content. #1

Merged
merged 1 commit into from
Dec 17, 2013
Merged

Use class_exists() guard for Featured_Content. #1

merged 1 commit into from
Dec 17, 2013

Conversation

tierra
Copy link
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
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
Member

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
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
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
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
Contributor Author

tierra commented Dec 17, 2013

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

@obenland
Copy link
Member

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

@tierra
Copy link
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.
leogermani pushed a commit that referenced this pull request Aug 12, 2022
When using WP-Cli to run the event "wp_cache_full_preload_hook" causes the follow error

> /webdir/bin/wp cron event run --due-now

```
PHP Fatal error:  Uncaught Error: Call to undefined function wp_cache_debug() in /webdir/web/app/plugins/wp-super-cache/wp-cache.php:3163
Stack trace:
#0 /webdir/web/wp/wp-includes/class-wp-hook.php(298): wp_cron_preload_cache()
#1 /webdir/web/wp/wp-includes/class-wp-hook.php(323): WP_Hook->apply_filters('', Array)
#2 /webdir/web/wp/wp-includes/plugin.php(515): WP_Hook->do_action(Array)
#3 phar:///webdir/bin/wp/php/commands/cron-event.php(284): do_action_ref_array('wp_cache_full_p...', Array)
#4 phar:///webdir/bin/wp/php/commands/cron-event.php(255): Cron_Event_Command::run_event(Object(stdClass))
#5 [internal function]: Cron_Event_Command->run(Array, Array)
#6 phar:///webdir/bin/wp/php/WP_CLI/Dispatcher/CommandFactory.php(67): call_user_func(Array, Array, Array)
#7 [internal function]: WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure}(Array, Array)
#8 phar:///webdir/bin/wp/php/WP_CLI/Dispatcher/Subcommand.php(372): call_user_func(Objec in /webdir/web/app/plugins/wp-super-cache/wp-cache.php on line 3163
Fatal error: Uncaught Error: Call to undefined function wp_cache_debug() in /webdir/web/app/plugins/wp-super-cache/wp-cache.php:3163
Stack trace:
#0 /webdir/web/wp/wp-includes/class-wp-hook.php(298): wp_cron_preload_cache()
#1 /webdir/web/wp/wp-includes/class-wp-hook.php(323): WP_Hook->apply_filters('', Array)
#2 /webdir/web/wp/wp-includes/plugin.php(515): WP_Hook->do_action(Array)
#3 phar:///webdir/bin/wp/php/commands/cron-event.php(284): do_action_ref_array('wp_cache_full_p...', Array)
#4 phar:///webdir/bin/wp/php/commands/cron-event.php(255): Cron_Event_Command::run_event(Object(stdClass))
#5 [internal function]: Cron_Event_Command->run(Array, Array)
#6 phar:///webdir/bin/wp/php/WP_CLI/Dispatcher/CommandFactory.php(67): call_user_func(Array, Array, Array)
#7 [internal function]: WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure}(Array, Array)
#8 phar:///webdir/bin/wp/php/WP_CLI/Dispatcher/Subcommand.php(372): call_user_func(Objec in /webdir/web/app/plugins/wp-super-cache/wp-cache.php on line 3163
```

This patch correct this based on deactivate/uninstall hooks which works on WP-Cli.
kraftbj added a commit that referenced this pull request Aug 23, 2022
…ing is deprecated in /wordpress/plugins/jetpack/11.3-a.7/functions.compat.php on line 94
kraftbj added a commit that referenced this pull request Aug 23, 2022
* PHP Deprecated: strpos(): Passing null to parameter #1 () of type string is deprecated in /wordpress/plugins/jetpack/11.3-a.7/functions.compat.php on line 94

* Add unit tests

* Update other functions

* Fix situations with arrays

* PHPCS exclusion for PHP 8.0 functions
samiff pushed a commit that referenced this pull request Sep 13, 2022
…tory-hook

Add after_wpsc_delete_cache_directory hook
jeherve pushed a commit that referenced this pull request Nov 16, 2023
Check for image before calling imagecreatefromstring()
iandunn added a commit that referenced this pull request Dec 7, 2023
This avoids a `ltrim(): Passing null to parameter #1 ($string) of type string is deprecated` notice with CLI commands on PHP 8.1.
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.

None yet

5 participants