Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#28284 closed defect (bug) (fixed)

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

Reported by: johnbillion's profile johnbillion Owned by: wonderboymusic's profile 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 10 years ago.
28284.2.diff (1.8 KB) - added by DrewAPicture 10 years ago.
docs

Download all attachments as: .zip

Change History (12)

#1 @DrewAPicture
10 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
10 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
10 years ago

#4 @kovshenin
10 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
10 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
10 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
10 years ago

  • Focuses docs added
  • Keywords dev-feedback removed

#8 @DrewAPicture
10 years ago

  • Keywords good-first-bug added

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

@DrewAPicture
10 years ago

docs

#9 @DrewAPicture
10 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
10 years ago

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