Opened 9 years ago
Closed 9 years ago
#34272 closed task (blessed) (fixed)
Audit embed function names
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Embeds | Keywords: | has-patch |
Focuses: | Cc: |
Description
In [34903] a number of functions relating to core's oEmbed implementation were introduced. Some of these functions use oembed
in their name, some use embed
in their name.
The following functions should be audited to determine whether any need to be renamed.
get_post_embed_url()
get_oembed_endpoint_url()
get_post_embed_html()
wp_oembed_excerpt_more()
the_excerpt_embed()
wp_oembed_excerpt_attachment()
Attachments (4)
Change History (21)
#2
@
9 years ago
The /embed/ rewrite endpoint isn't really dependant on oEmbed. That's why naming get_post_embed_url()
, get_post_embed_html()
and the_excerpt_embed()
like that makes sense.
I suggest renaming wp_oembed_excerpt_more()
and wp_oembed_excerpt_attachment()
to use a wp_embed
prefix (or perhaps just embed
). There are also print_oembed_embed_styles()
and print_oembed_embed_scripts()
which could be renamed to print_embed_styles()
and print_embed_scripts()
.
In the embed template, there are hooks like oembed_head
and oembed_footer
. From what I said above, we should consider using embed_head
and embed_footer
there.
An alternative approach is to just use oembed
everywhere, if that's not too confusing.
#3
@
9 years ago
- Keywords needs-patch added; dev-feedback removed
My preference is to favour embed
over oembed
when the functionality isn't specific to oEmbed. I think all of the above can use embed
instead of oembed
.
#4
@
9 years ago
The get_post_embed_html()
function includes some JavaScript from js/wp-oembed.js
. Is this JavaScript oEmbed-specific? If so, is get_post_embed_html()
oEmbed-specific?
#5
@
9 years ago
My preference is to favour embed over oembed when the functionality isn't specific to oEmbed.
Likewise.
If so, is get_post_embed_html() oEmbed-specific?
No. It can be used without having an oEmbed endpoint at all. Since it includes the wp-oembed.js, both aren't 100% oEmbed-specific.
#6
@
9 years ago
- Keywords has-patch added; needs-patch removed
34272.diff renames files and functions. This makes it more clear what is oEmbed-specific and what isn't.
The renamed files need to be moved with svn cp
when committing though.
#7
@
9 years ago
- Keywords commit added
For reference:
= Files = wp-includes/css/wp-oembed-embed.css => wp-includes/css/wp-embed-template.css wp-includes/js/wp-oembed-embed.js => wp-includes/js/wp-embed-template.js wp-includes/js/wp-oembed.js => wp-includes/js/wp-embed.js = Functions = print_oembed_embed_scripts() => print_embed_scripts() wp_oembed_excerpt_more() => wp_embed_excerpt_more() wp_oembed_excerpt_attachment() => wp_embed_excerpt_attachment() print_oembed_embed_styles() => print_embed_styles() print_oembed_embed_scripts() => print_embed_scripts() = Hooks = oembed_html => embed_html oembed_head => embed_head oembed_thumbnail_image_size => embed_thumbnail_image_size oembed_thumbnail_image_shape => embed_thumbnail_image_shape oembed_content => embed_content oembed_site_icon_url => embed_site_icon_url oembed_content_meta => embed_content_meta oembed_site_icon_url => embed_site_icon_url oembed_footer => embed_footer = Tests = test_wp_oembed_excerpt_more_no_embed() => test_wp_embed_excerpt_more_no_embed() test_wp_oembed_excerpt_more() => test_wp_embed_excerpt_more()
#10
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
In 34272.2.diff, fix for enqueueing and printing wp-embed.js
missed in original commit.
#12
@
9 years ago
- Keywords needs-refresh added; commit removed
Running a quick refresh, I missed oembed_foot
=> embed_foot
on L459 in default filters.php?
#16
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
The TinyMCE wpoembed
plugin should be renamed to wpembed
as well (including the folder name). See 34272.4.diff .
@swissspidy are you able to take a look?