-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Move wp_oembed_add_discovery_links() up to print at wp_head priority 4
#10449
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
Move wp_oembed_add_discovery_links() up to print at wp_head priority 4
#10449
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
swissspidy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is what it takes to maintain back compat... I just hope we never have to change the priority again :-)
This comment was marked as off-topic.
This comment was marked as off-topic.
|
I've opened #10456 to introduce the |
… trac-64178-oembed-discovery-link-position
…into trac-64178-oembed-discovery-link-position * 'trunk' of https://github.com/WordPress/wordpress-develop: Plugins: Add missing `$priority` parameter to `has_filter()` and `has_action()`.
…ad multiple times
…ead` priority 4. This results in the oEmbed discovery links being printed before scripts and styles are printed. This helps ensure they appear within the first 150K bytes of the HTML response, which is required by `WP_oEmbed::discover()`. With increasing the `styles_inline_size_limit` from 20K to 40K in #63018, it becomes more probable that the oEmbed discovery links will be pushed out of range. For backwards compatibility with themes and plugins that disable the oEmbed discovery links by unhooking `wp_oembed_add_discovery_links()` from running at `wp_head` priority 10 (even though the `oembed_discovery_links` filter is available to disable such links), this callback is added a second time to run earlier at priority 4. The first time the function runs, it checks to see if the callback is still hooked at priority 10. If not, the function short circuits. If it is still hooked at priority 10, however, the function then unhooks itself at priority 10 so that it won't run a second time later during the `wp_head` action, before proceeding with printing the discovery links. A similar back-compat approach was taken previously in [60910]. The back-compat checks are only performed if the function is invoked during the `wp_head` action, allowing the function to be called "idempotently" elsewhere. Developed in #10449 Follow-up to [61118], [61117]. Props westonruter, swissspidy. See #64186, #63018. Fixes #64178. git-svn-id: https://develop.svn.wordpress.org/trunk@61119 602fd350-edb4-49c9-b593-d223f7449a82
…ead` priority 4. This results in the oEmbed discovery links being printed before scripts and styles are printed. This helps ensure they appear within the first 150K bytes of the HTML response, which is required by `WP_oEmbed::discover()`. With increasing the `styles_inline_size_limit` from 20K to 40K in #63018, it becomes more probable that the oEmbed discovery links will be pushed out of range. For backwards compatibility with themes and plugins that disable the oEmbed discovery links by unhooking `wp_oembed_add_discovery_links()` from running at `wp_head` priority 10 (even though the `oembed_discovery_links` filter is available to disable such links), this callback is added a second time to run earlier at priority 4. The first time the function runs, it checks to see if the callback is still hooked at priority 10. If not, the function short circuits. If it is still hooked at priority 10, however, the function then unhooks itself at priority 10 so that it won't run a second time later during the `wp_head` action, before proceeding with printing the discovery links. A similar back-compat approach was taken previously in [60910]. The back-compat checks are only performed if the function is invoked during the `wp_head` action, allowing the function to be called "idempotently" elsewhere. Developed in WordPress/wordpress-develop#10449 Follow-up to [61118], [61117]. Props westonruter, swissspidy. See #64186, #63018. Fixes #64178. Built from https://develop.svn.wordpress.org/trunk@61119 git-svn-id: http://core.svn.wordpress.org/trunk@60455 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…ead` priority 4. This results in the oEmbed discovery links being printed before scripts and styles are printed. This helps ensure they appear within the first 150K bytes of the HTML response, which is required by `WP_oEmbed::discover()`. With increasing the `styles_inline_size_limit` from 20K to 40K in #63018, it becomes more probable that the oEmbed discovery links will be pushed out of range. For backwards compatibility with themes and plugins that disable the oEmbed discovery links by unhooking `wp_oembed_add_discovery_links()` from running at `wp_head` priority 10 (even though the `oembed_discovery_links` filter is available to disable such links), this callback is added a second time to run earlier at priority 4. The first time the function runs, it checks to see if the callback is still hooked at priority 10. If not, the function short circuits. If it is still hooked at priority 10, however, the function then unhooks itself at priority 10 so that it won't run a second time later during the `wp_head` action, before proceeding with printing the discovery links. A similar back-compat approach was taken previously in [60910]. The back-compat checks are only performed if the function is invoked during the `wp_head` action, allowing the function to be called "idempotently" elsewhere. Developed in WordPress/wordpress-develop#10449 Follow-up to [61118], [61117]. Props westonruter, swissspidy. See #64186, #63018. Fixes #64178. Built from https://develop.svn.wordpress.org/trunk@61119 git-svn-id: https://core.svn.wordpress.org/trunk@60455 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This addresses an issue whereby if there is a lot of markup printed at
wp_head(e.g. inline styles from Core-63018), WordPress sites looking for the oEmbed discovery links will fail to find them because they only read the first 150KB of the HTML response, by default:wordpress-develop/src/wp-includes/class-wp-oembed.php
Line 444 in bafd51b
Note that scripts and styles are printed at priorities 8 and 9:
wordpress-develop/src/wp-includes/default-filters.php
Lines 355 to 356 in bafd51b
In order to ensure the oEmbed links remain discoverable, it seems prudent to print them early, specifically right after other similar links are printed, namely
feed_links()andfeed_links_extra().It is not so simple as changing the action priority from 10 to 4:
wordpress-develop/src/wp-includes/default-filters.php
Line 707 in bafd51b
This is because there are many plugins which remove the oEmbed discovery links via:
See GitHub Search (1.7k files) and WPDirectory (145 plugins).
Aside: The better way to do this would be to do:
In order to maintain backwards-compatibility with the ecosystem which unhooks the action in this way, it is necessary to add a second instance of
wp_oembed_add_discovery_linksrunning atwp_headwith the earlier priority of 4. Inside ofwp_oembed_add_discovery_links(), when it runs first at priority 4, it can check if it has been unhooked at priority 10, and if so, short-circuit. Otherwise, it can continue with printing the oEmbed discovery links, but also preventing them from being duplicated by itself doing:In order to facilitate this, it is necessary to extend the
has_action()/has_filter()functions with a new$priorityparameter to be able to check whether a given callback is added at a specific priority. This is something I've missed in the past. It also "feels right" given thatadd_action()/remove_action()/add_filter()/remove_filter()all have an optional$priorityparameter. See examples on GitHub and on WP Directory where ecosystem code assumed there was such a$priorityparameter. This is important because the same callback can be added multiple times on the same hook with different priorities.A similar approach to backwards-compatibility has been done recently, with
wp_print_auto_sizes_contain_css_fix()in example r60910. The difference there is that a new function was introduced, whereas here the function remains the same.TODO
Trac ticket: https://core.trac.wordpress.org/ticket/64178
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.