[wp-trac] [WordPress Trac] #42918: Replace intval(), strval(), ... function calls with type casts
WordPress Trac
noreply at wordpress.org
Fri Oct 9 05:24:06 UTC 2020
#42918: Replace intval(), strval(), ... function calls with type casts
-------------------------+-----------------------------
Reporter: ayeshrajans | Owner: SergeyBiryukov
Type: enhancement | Status: closed
Priority: normal | Milestone: 5.6
Component: General | Version:
Severity: normal | Resolution: fixed
Keywords: has-patch | Focuses: performance
-------------------------+-----------------------------
Comment (by johnjamesjacoby):
Hey @SergeyBiryukov!
r49108 is causing some breakage for me. I think the related code change
was unintended.
The Sugar Calendar plugin has used this approach for adding its Top Level
menu page for a few years now:
{{{
add_menu_page(
esc_html__( 'Calendar', 'sugar-calendar' ),
esc_html__( 'Calendar', 'sugar-calendar' ),
'read_calendar',
'sugar-calendar',
'Sugar_Calendar\\Admin\\Menu\\calendar_page',
'dashicons-calendar-alt',
2
);
}}}
Note the position: `2`. It matches the position of core's "Dashboard" menu
item.
* Before this change, WordPress would automatically reassign a float
position of `2.06668` and put our "Calendar" item directly below
"Dashboard" ✅
* After this change, it gets set to `2` and replaces "Dashboard" 💥
The flaw in r49108 is that:
{{{
$menu[ "$position" ] = $new_menu;
}}}
...was changed to...
{{{
$menu[ $position ] = $new_menu;
}}}
PHP doesn't allow floats to be array keys, but it will allow strings to
be, and WordPress is (perhaps unintentionally) smart enough to continue to
order ints and strings-with-float-contents in the correct numerical
sequence.
I'd recommend reverting that line, perhaps to this to be even more clear
that this is intentional:
{{{
$menu[ "{$position}" ] = $new_menu;
}}}
Thoughts?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/42918#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list