[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