Opened 7 years ago
Closed 7 years ago
#42310 closed defect (bug) (fixed)
Inconsistent update and refresh logic between `oembed_cache` and post meta
Reported by: | dlh | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Embeds | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
The logic for caching oEmbeds in the oembed_cache
post type seems to have a couple of inconsistencies with the existing logic for caching oEmbeds in post meta.
First, there isn't logic in WP_Embed::shortcode()
to update to an existing oembed_cache
post. If WP_Embed::usecache
is false
and the embed's TTL has passed, then WP_Embed::shortcode()
will insert a new oembed_cache
post even when a $cached_post_id
exists. By contrast, for post meta, any existing _oembed_
and _oembed_time_
values are updated.
One part of the attached patch adds logic to use wp_update_post()
when there is a $cached_post_id
. Per [28972], one of the goals of the patch is also to not overwrite a previously valid cache with {{unknown}}
.
Second, there isn't logic to refresh caches of oEmbed in widgets after saving the widget. By contrast, when a post is saved in the admin, WP_Embed::maybe_run_ajax_cache()
will print JavaScript on the next pageload that refreshes the embeds in the post content via the oembed-cache
Ajax action.
This inconsistency seems a little tougher to rectify because widgets are already saved over Ajax. They're also saved using different mechanisms on widgets.php
and customize.php
. However, it's arguably more important to refresh oembed_cache
embeds because they're saved "globally" instead of specifically to a single post. If an oEmbed fails in one widget, it will fail for all future widgets.
The attached patch would address this issue in wp_ajax_parse_embed()
. It would set WP_Embed::usecache
to false
when there is no post ID (which is the case with embeds in widgets). Effectively, an embed whose TTL had passed would be refreshed as part of fetching it to display in the widget form.
This approach has the advantage of occurring automatically for users once they open a widget form on either widgets.php
or customize.php
. But as @adamsilverstein pointed out to me at WordCamp NYC today, it breaks with existing user expectations in that saving the widget wouldn't itself cause anything to refresh, unlike the current behavior with posts.
In that sense, methods for refreshing oembed_cache
posts after saving changes on widgets.php
or customize.php
might be preferable.
@dlh:
This is a challenge. While this could maybe align with user expectations for embeds in widgets specifically, it wouldn't then help with using embeds in TinyMCE outside of the widget context. For example, if I add a TinyMCE editor to the user profile screen and I want to drop an embed in there, we wouldn't be able to reliably know when to refresh the oEmbed other than there also attempt to manually add a refresh when user updated action fires. So I think it is useful to piggyback on the
parse-embed
request to also refresh the cache.