Opened 10 years ago
Closed 10 years ago
#32894 closed defect (bug) (fixed)
Customizer Menus: page enters an infinite reloading loop when using a menu with a walker
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.3 | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Customize | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
The problem occurs in the Customize -> Menus section with WP 4.3 Beta 1 - the customized page enters an infinite reloading loop when using a menu with a walker. In order to reproduce it:
- The page that is being customized has to load the menu with the walker option, for example:
wp_nav_menu( array( 'menu_class' => 'nav-menu', 'theme_location' => 'primary', 'walker' => new Custom_Walker_Nav_Menu ) );
- Following the code from the example above, before editing the menu, assign it to the primary theme location , so that the menu is loaded on the page that is being customized
- Reorder the menu items and the page should enter an infinite reloading loop
The issue happens regardless of the code of the custom walker class - I have tested it with an empty class and also with the code from the example here: https://codex.wordpress.org/Function_Reference/wp_nav_menu#Using_a_custom_Walker_class
I have also tested this with different themes and no plugins installed, it happens with the TwentyFifteen theme, when I modify the menu code in the sidebar.php file.
Attachments (3)
Change History (19)
#1
@
10 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 4.3
- Owner set to westonruter
- Status changed from new to accepted
#3
in reply to:
↑ 2
@
10 years ago
Replying to westonruter:
I cannot reproduce the problem here, neither on 4.3 beta 1 or on trunk [...]
That's strange, I created a second installation on my localhost to test that and the same problem happened. I've been using the Beta Tester plugin that has updated the installation to 4.3-beta1-33088.
I've been trying to trace the problem, so far I found that in the customize-preview-nav-menus.js file, when the refreshMenuInstance()
function is called and the walker option is set, the container element:
container = $( '#partial-refresh-menu-container-' + String( instanceNumber ) );
doesn't exist (while it exists without the walker option set)
and then the following code is executed
if ( ! instance.can_partial_refresh || 0 === container.length ) { api.preview.send( 'refresh' ); return; }
which might be related with the looping issue.
I hope that someone else can reproduce the issue, but meanwhile I'll keep on digging - I'll keep you posted.
#4
@
10 years ago
I had a similar issue, but without using Custom Walker. I was unable to trace the steps to reproduce the problem, sometime it does and sometimes not.
I will do more tests here and try to find something.
This ticket was mentioned in Slack in #core-customize by sheri. View the logs.
10 years ago
#6
@
10 years ago
Unfortunately I couldn't find what was causing the issue - I tried to trace the JS code, but since I wasn't familiar with it, I got lost between all of the fired events and couldn't find the one that was initializing the looping issue.
I found that what I mentioned above (the non-existing container
element) was happening because the can_partial_refresh
option is by default set to false when a walker is used (in the filter_wp_nav_menu_args()
function of the wp-includes/class-wp-customize-nav-menus.php file) , but I guess that's how it's meant to be.
I also tested this on two more installations - I created another new installation on my localhost and also tried on another installation on another computer - the same issue happened every time.
I have attached the modified TwentyFifteen theme that I have been using to test - it has the walker option set in the sidebar.php file and also contains a walker class in the functions.php file, so if you want you can give it a try.
If you test it, please make sure that the menu that's being edited is assigned as a primary menu so that it is displayed on the page, as the issue doesn't happen when the menu is not assigned.
If you want me to provide any additional info or try some additional testing on my installation, please let me know - I'll be happy to help.
#7
@
10 years ago
I was able to reproduce the problem now. It looks like the problem is rooted in wp.customize.menusPreview.refreshMenuInstanceDebounced()
. When adding a new nav menu item, the type_label
property is set whereas in the Customizer preview the settings get exported without this property. The result is that when the preview first loads up and the Customizer re-syncs the settings into the Preview, it detects a change and then triggers an update. Still working on a fix.
#8
@
10 years ago
- Keywords has-patch needs-testing added; reporter-feedback removed
OK, please check out 32894.diff. It defers the listening to nav menu setting changes until active
, that is until after the Customizer has synced settings into the preview. This ensures that any differences between the JS and PHP representations of the settings won't cause an infinite refresh.
#9
follow-ups:
↓ 10
↓ 11
@
10 years ago
I have just tested 32894.diff - it works well for me, the infinite refreshing is not happening anymore.
However the walker is not applied to the menus in the customizer (it is only applied on the front-end), is this a known issue?
#10
in reply to:
↑ 9
@
10 years ago
Replying to dennis_f:
I have just tested 32894.diff - it works well for me, the infinite refreshing is not happening anymore.
However the walker is not applied to the menus in the customizer (it is only applied on the front-end), is this a known issue?
The Customizer doesn't support the nav menu walker setup that the admin screen has for a variety of reasons. Instead, we're adding hooks that make the system easier to extend and make cross-compatible. See #32832.
#11
in reply to:
↑ 9
;
follow-up:
↓ 12
@
10 years ago
- Keywords needs-testing removed
Replying to dennis_f:
However the walker is not applied to the menus in the customizer (it is only applied on the front-end), is this a known issue?
Yes, this is being addressed in #32781. Please test with 32894-with-32781.diff which combines the patch from this ticket with the patch from that ticket.
#12
in reply to:
↑ 11
@
10 years ago
Replying to westonruter:
Replying to dennis_f:
However the walker is not applied to the menus in the customizer (it is only applied on the front-end), is this a known issue?
Yes, this is being addressed in #32781. Please test with 32894-with-32781.diff which combines the patch from this ticket with the patch from that ticket.
That works well for me too - with those fixes, the walker code works in the preview and the page refreshes normally, just once.
I just couldn't run the tests changes from the tests/phpunit/tests/customize/nav-menus.php file as I don't have the tests on my setup.
I cannot reproduce the problem here, neither on 4.3 beta 1 or on trunk. I used that
themeslug_walker_nav_menu
class from the codex page, putting it infunctions.php
for Twenty Fifteen, and then modified the primary nav menu to be embedded via:But upon changing a nav menu item by reordering or changing a property, the preview does a full refresh as expected in that no apparent infinite loop is experienced.
Note I did find one unexpected thing, however: the custom walker is dropped when the menu is previewed in the preview. This is due to a logic problem in
WP_Customize_Nav_Menus::filter_wp_nav_menu_args()
. I've re-opened #32781 with a patch to fix that issue.But otherwise, I can't reproduce the infinite loop problem. Please advise.