Make WordPress Core

Opened 7 months ago

Closed 7 months ago

#63248 closed defect (bug) (fixed)

PHPDoc for embed_handler_html is misleading

Reported by: apermo's profile apermo Owned by: audrasjb's profile audrasjb
Milestone: 6.8 Priority: normal
Severity: normal Version: 2.9
Component: Embeds Keywords: docs has-patch commit dev-reviewed fixed-major
Focuses: Cc:

Description

<?php
( false !== $return ) {
        /**
         * Filters the returned embed HTML.
         *
         * @since 2.9.0
         *
         * @see WP_Embed::shortcode()
         *
         * @param string|false $return The HTML result of the shortcode, or false on failure.
         * @param string       $url    The embed URL.
         * @param array        $attr   An array of shortcode attributes.
         */
        return apply_filters( 'embed_handler_html', $return, $url, $attr );
}

Similar argument as in #63220, but even easier.

While the documentation suggests that $return can be false or a string, it can't be false, we are testing exactly for that.

This is misleading.

I've checked the whole file's filters this time, all others look good to me, no actions in that file.

PR follows.

Change History (8)

This ticket was mentioned in PR #8661 on WordPress/wordpress-develop by @apermo.


7 months ago
#1

  • Keywords has-patch added

docs: Fixed parameter type and descriptions for embed_handler_html filter.

Trac ticket: https://core.trac.wordpress.org/ticket/63248#ticket

@apermo commented on PR #8661:


7 months ago
#2

Note for reviewers: Hit expand to top once.

You'll see that there is a condition right before the PHPDoc saying false !== $return

#3 @SergeyBiryukov
7 months ago

  • Milestone changed from Awaiting Review to 6.9

#4 @SergeyBiryukov
7 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 60149:

Docs: Correct the type of the $return parameter in embed_handler_html filter.

The value cannot be false at this point, as the filter is only applied to non-empty embed HTML.

Follow-up to [25726], [46661].

Props apermo, jasonsa19.
Fixes #63248.

#5 @SergeyBiryukov
7 months ago

  • Keywords commit dev-feedback added
  • Milestone changed from 6.9 to 6.8
  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks for the ticket! Reopening for 6.8.x consideration to complement [60140] / #63220.

#6 @audrasjb
7 months ago

  • Keywords dev-reviewed fixed-major added; dev-feedback removed

Marking as ready for 6.8 backport as #63220 was backported last week.

Last edited 7 months ago by audrasjb (previous) (diff)

#7 @audrasjb
7 months ago

  • Owner changed from SergeyBiryukov to audrasjb
  • Status changed from reopened to assigned

Self assigning for backport.

#8 @audrasjb
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 60157:

Docs: Correct the type of the $return parameter in embed_handler_html filter.

The value cannot be false at this point, as the filter is only applied to non-empty embed HTML.

Follow-up to [25726], [46661].

Reviewed by SergeyBiryukov, audrasjb.
Merges [60149] to 6.8 branch.
Props apermo, jasonsa19.
Fixes #63248.

Note: See TracTickets for help on using tickets.