WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#28284 closed defect (bug) (fixed)

Calling `wp_oembed_add_provider()` before the `plugins_loaded` action shouldn't be allowed

Reported by: johnbillion Owned by: wonderboymusic
Milestone: 4.0 Priority: low
Severity: major Version: 2.9
Component: Embeds Keywords: has-patch
Focuses: docs Cc:

Description

If a plugin calls wp_oembed_add_provider() before the plugins_loaded hook (ie. as soon as the plugin file is included), it can prevent other plugins from hooking into the oembed_providers filter.

This happens because wp_oembed_add_provider() calls _wp_oembed_get_object() which instantiates the WP_oEmbed class which immediately applies the oembed_providers filter in its constructor. If another plugin is hooked into this filter but isn't yet loaded, it's straight outta luck because the filter has already been applied.

To prevent this problem, we should add a check to wp_oembed_add_provider() which triggers a _doing_it_wrong() call if the plugins_loaded action hasn't fired. This gives other plugins a chance to hook into the oembed_providers filter.

Alternatively we could move the oembed_providers filter so it fires JIT (in the get_html() method) rather than when the class is instantiated.

Attachments (2)

28284.diff (2.7 KB) - added by kovshenin 8 years ago.
28284.2.diff (1.8 KB) - added by DrewAPicture 7 years ago.
docs

Download all attachments as: .zip

Change History (12)

#1 @DrewAPicture
8 years ago

  • Milestone changed from Awaiting Review to 4.0

Seems sensible.

Not sure about moving the filter though. Seems like that might unnecessarily invite problems for plugins already leveraging it. Might just be worth moving it early to give plugin devs plenty of time to raise issues.

#2 @lukbarros
8 years ago

  • Severity changed from normal to major

Hello this problem seems valid, I've had this problem before, when I make a plugin called this method / function wp_oembed_add_provider () in my plugin that was the first to carry on my Musted Used plugins (MU-PLUGINS) and other third party installed normally (PLUGINS) stopped working as well.

I have not analyzed the code but the description seems quite valid alternative to isolate plugins possão not call wp_oembed_add_provider () directly, it would need to go trough a hook filter type, you can then add providers without affecting the scope of operation of other plugins, this can be a knife to the neck of a planter riding based on third-party plugins, and somehow this makes as developers of plugins to use hooks as expected.

I'm relatively new here, I would analyze and fix it, how to proceed?

@kovshenin
8 years ago

#4 @kovshenin
8 years ago

  • Keywords has-patch added

In 28284.diff: if add/remove helper functions called before plugins_loaded, don't instantiate the oembed object, but rather use a static class variable to hold "just in time" additions and removals, which are then executed when the object is actually created.

#5 @wonderboymusic
7 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 28846:

When wp_oembed_add_provider() or wp_oembed_remove_provider() is called before the plugins_loaded hook has, store the values statically on the WP_oEmbed object and add them just-in-time when the object is instantiated.

This ensures that all plugins have an accurate provider list when apply_filters( 'oembed_providers', $providers ) is called.

Props kovshenin.
Fixes #28284.

#6 @DrewAPicture
7 years ago

  • Keywords needs-docs added
  • Resolution fixed deleted
  • Status changed from closed to reopened

At the very least, we need basic docblocks on any new functions or methods, though the more complete the better. See WP_oEmbed::_add_provider_early() and WP_oEmbed::_remove_provider_early().

#7 @wonderboymusic
7 years ago

  • Focuses docs added
  • Keywords dev-feedback removed

#8 @DrewAPicture
7 years ago

  • Keywords good-first-bug added

Marking good-first-bug for WC Seattle contributor day. See comment:6.

@DrewAPicture
7 years ago

docs

#9 @DrewAPicture
7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 29012:

Add phpDoc blocks for two WP_oEmbed methods added in [28846].

Fixes #28284.

#10 @DrewAPicture
7 years ago

  • Keywords needs-docs good-first-bug removed
Note: See TracTickets for help on using tickets.