[wp-trac] [WordPress Trac] #39692: Fix missing assignment of menus on theme switch

WordPress Trac noreply at wordpress.org
Fri Aug 4 18:26:43 UTC 2017


#39692: Fix missing assignment of menus on theme switch
-------------------------------------------------+-------------------------
 Reporter:  melchoyce                            |       Owner:
     Type:  enhancement                          |      Status:  assigned
 Priority:  normal                               |   Milestone:  4.9
Component:  Menus                                |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch needs-testing needs-unit-  |     Focuses:
  tests                                          |
-------------------------------------------------+-------------------------

Comment (by welcher):

 I reviewed the patch and just have a couple of comments.

 1. I think we should do the check for `! empty( $old_nav_menu_locations )
 ` right away and only do the processing if so. It's a minor thing, but why
 do any processing if there is nothing to convert from.
 2. The `in_array` check is not doing a strict check. Technically, nav
 menus can be registered with a numeric location such as `array( 1 =>
 'First Menu', 2 => 'Second Menu', )` which will produce a false positive
 and in turn, an undefined offset error.

 I have applied the patch and am getting a failure on one of the unit
 tests:
 {{{
 1) Tests_Nav_Menu_Theme_Change::test_one_location_each

 Failed asserting that two arrays are equal.
 --- Expected
 +++ Actual
 @@ @@
  Array (
 +    0 => 1
  )
 }}}

 I've attached a patch with the proposed changes and a new test that
 demonstrates the `in_array` error if not using strict.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39692#comment:43>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list