#59613 closed defect (bug) (duplicate)
Ensure $atts passed to $callback of add_shortcode() is always an array
Reported by: | bedas | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Shortcodes | Keywords: | has-patch 2nd-opinion dev-feedback |
Focuses: | Cc: |
Description
Problem
According to the documentation:
$callback
callable Required
The callback function to run when the shortcode is found.
Every shortcode callback is passed three parameters by default, including an array of attributes ($atts
), the shortcode content or null if not set ($content
), and finally the shortcode tag itself ($shortcode_tag
), in that order.
However, this part...
including an array of attributes (
$atts
)
...is not always true:
- if the shortcode is used without any attributes, then
$atts
is actually an empty string. - apparently (according to code docbloc) it would also be a string of original arguments if it could not be parsed.
This discrepancy causes other dependant code to always fumble with the type of $atts
, for example the source code of shortcode_atts has to typecasts $atts
to array.
This would not be necessary, would $atts
indeed be always an array from the start.
Further, in more complex programming scenarios where we assume $atts
to be an array (as stated by the documentation), again we have to fumble with $atts
, and in some cases, we cannot do anything but just accept that it may or may not be an array.
You can read more on that here
Cause
Thanks to the kind help of @bcworkz and further testing, I can confirm that the issue is within shortcode_parse_atts
function - to be precise in the else
statement here.
If the checks for if ( preg_match_all( $pattern, $text, $match, PREG_SET_ORDER ) )
fail, $atts
is transformed to string using ltrim
.
This makes thus poor sense to me:
- First of all, the documentation mentions that
$atts
is an array. It never mentions other possible types. - It seem quite clear to me that
$atts
is always supposed and intended to be an array - never a string. - Even if the docblock for
shortcode_parse_atts
states that the returned value can be array or string (and if string, then theoriginal arguments string if it cannot be parsed
)... I do not see the actual reason or justification for this.
What is the usefulness of returning a string of arguments if it cannot be parsed or is empty?
It should then safely fail into an empty array, to keep consistency and not force dependant code to typecast $atts
.
Proposal
Simply remove the else
block in shortcode_parse_atts
.
It is redundant and creates issues in particular coding structures + clear discrepancies in documentation.
Change History (3)
This ticket was mentioned in PR #5480 on WordPress/wordpress-develop by @bedas.
11 months ago
#1
#2
@
11 months ago
- Focuses docs performance sustainability php-compatibility removed
- Milestone Awaiting Review deleted
- Resolution set to duplicate
- Status changed from new to closed
Duplicate of #59249.
@swissspidy commented on PR #5480:
7 months ago
#3
Committed in https://core.trac.wordpress.org/changeset/57597
Ensures
$atts
passed to$callback
ofadd_shortcode()
is always an array