Opened 3 months ago
Closed 2 months ago
#63220 closed defect (bug) (fixed)
Documentation for embed_oembed_html misleading
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | 2.9 |
Component: | Embeds | Keywords: | has-patch dev-feedback |
Focuses: | docs | Cc: |
Description
<?php if ( ! empty( $cache ) ) { /** * Filters the cached oEmbed HTML. * * @since 2.9.0 * * @see WP_Embed::shortcode() * * @param string|false $cache The cached HTML result, stored in post meta. * @param string $url The attempted embed URL. * @param array $attr An array of shortcode attributes. * @param int $post_id Post ID. */ return apply_filters( 'embed_oembed_html', $cache, $url, $attr, $post_id ); }
<?php if ( $html ) { /** This filter is documented in wp-includes/class-wp-embed.php */ return apply_filters( 'embed_oembed_html', $html, $url, $attr, $post_id ); }
While the documentation suggests that $cache
/$html
can be false
or a string
, it can't be false
and even not an empty string.
This is misleading, we wanted to create a filter for that and added type hints to our filter and were unsure about the type. After reading the code we found that it was misleading.
I will open a PR for this.
Change History (8)
This ticket was mentioned in PR #8640 on WordPress/wordpress-develop by @apermo.
3 months ago
#1
- Keywords has-patch added
#4
@
3 months ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 60127:
#5
@
3 months ago
- Keywords dev-feedback added
- Milestone changed from 6.9 to 6.8
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening and moving to the 6.8 milestone for r60127 to be considered for merging to the 6.8 branch.
#6
@
3 months ago
This really reduces confusion and potential misuse, especially for the developers who works on custom embed behavior.
#7
@
3 months ago
That confusion is exactly what lead me and @xate to find this. We were discussing which type hints we needed and since PHPdoc was suggesting that false
could be a valid value, we were about to make our code more complicated that it needed to be.
So fixing this, will potentially have developers create less complex code on their side. I'd highly welcome it in 6.8.
Since it is only a PHPdoc fix, it has no potential of breaking the update, I don't see any reason to hold it back till 6.9.
If it is to short of a notice for 6.8, maybe it could be considered for 6.8.1?
As described in the trac ticket, the PHPdoc for
embed_oembed_html
is misleading. This fixes this issue.Trac ticket: https://core.trac.wordpress.org/ticket/63220#ticket