Opened 9 years ago
Closed 9 years ago
#35112 closed defect (bug) (fixed)
Screen Options in Appearance -> Menu not saved correctly sometimes
Reported by: | Asgaros | Owned by: | afercia |
---|---|---|---|
Milestone: | 4.4.1 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Administration | Keywords: | has-patch fixed-major |
Focuses: | ui, javascript | Cc: |
Description
Hello everyone!
I found a strange behavior when I want to change the "Screen Options" in the "Appearance -> Menus" area inside the administration interface.
Lets assume that in the first step all checkboxes are selected:
Now I deselect all "Boxes" checkboxes:
After that I leave the "Appearance -> Menus" page (lets go to the dashboard for example) and go back to it again.
When I try to unselect one of the "Show advanced menu properties" checkboxes like the "Title Attribute", leave the "Appearance -> Menus" page and go back to it, the checkbox is selected again. Sometimes even all of the "Boxes" checkboxes are selected again.
It is not always possible to reproduce it because the problem does not occur everytime and seems to be a little bit random. Sometimes if I unselect all "Boxes" checkboxes and come back to the page, some of them are selected again. Another example: If I select two/three of the "Boxes" checkboxes after all of those checkboxes were unselected and refresh the page, all or some of the not-selected "Boxes" are selected as well.
I tested this with version 4.4/trunk on different sites with different browsers (Chrome/Edge) and different themes (default and own).
I hope someone can reproduce this problem too. If not I can try to upload some screen capture later.
Attachments (2)
Change History (21)
#3
@
9 years ago
- Component changed from Menus to Administration
- Focuses ui javascript added; administration removed
- Milestone changed from Awaiting Review to 4.4.1
#4
follow-up:
↓ 5
@
9 years ago
Additionally, as far as I see, in the Menus screen the "Boxes" checkboxes have 2 click events attached. The first one from postbox.js
by postboxes.add_postbox_toggles()
and the second one from nav-menus.js
by wpNavMenu.initToggles()
. When the first one is triggered, the AJAX call in the second one is triggered too but the ID values are not get yet. So there are always 2 AJAX calls and the first one has empty parameters.
This happens also in previous WP versions and shouldn't be directly related to the reported bug.
#5
in reply to:
↑ 4
@
9 years ago
Replying to afercia:
When the first one is triggered, the AJAX call in the second one is triggered too
Almost :) Actually, both functions trigger an AJAX call, see postboxes.save_state()
#6
@
9 years ago
- Keywords has-patch added
In the proposed patch:
- On the Screen Options in the Menus screen, restore the checkboxes IDs removed in [34991]. Also, avoid a useless AJAX call.
Some testing would be greatly appreciated :)
#7
@
9 years ago
@afercia Thanks for the patch! The Screen options in the Appearance -> Menu area are working great with it.
But I think I found another related problem. Here are the relevant steps to reproduce it:
Step 1: Lets assume that in the first step all checkboxes are selected again.
Step 2: Click on Manage in Customizer
Step 3: Click the settings icon. All checkboxes are selected. So far so good.
Step 4: Now de-select one checkbox, refresh the page and click the settings icon again. All checkboxes are selected again now.
If I change checkboxes more than one time in this step, everything is saved correctly. It looks like the first AJAX request is missing some parameters/data.
Example:
When i de-select Link Target in Step 4 this request-data will be sent:
But it should send this request-data (which will be sent when I un-select, select and un-select the checkbox again):
I hope it helps! :)
#8
@
9 years ago
I modified the hidden-function in the customize-nav-menus.js a little bit. The
this.hidden = function() { // ... };
wrapper prevented it from working. More testing would be great.
#9
@
9 years ago
@Asgaros thanks very much for your thorough test :)
Yep looks like a separate issue in customize-nav-menus.js
since when clicking the first time on a checked checkbox, the returned value is undefined instead of an ID. This should probably go in a separate ticket but I will defer to @westonruter :)
This ticket was mentioned in Slack in #core-customize by afercia. View the logs.
9 years ago
#11
@
9 years ago
@Asgaros @afercia I cannot reproduce the issue. With 35112.2.patch applied, I am able to uncheck one checkbox and reload and see that one checkbox still unchecked: https://cloudup.com/cyH16zBhoVD
#12
@
9 years ago
@westonruter Yep, 35112.2.patch seems to fix it by the way the first bug is a regression from 4.4 and the second one is unrelated, probably has worked this way forever. Not sure why it was coded that way.
Just to clarify, with the first patch applied which fixes just the regression, here's what happens for me. See screenshot, dumping hidden
in customize-nav-menus.js
at line 616.
The first time I click on "Link Target", hidden
is undefined, the second time I click on "Title Attribute" and it correctly returns both IDs.
This ticket was mentioned in Slack in #core by jorbin. View the logs.
9 years ago
#15
@
9 years ago
35112.patch looks good for 4.4.1 here too.
35112.2.patch should probably go in 4.5. There is another typo in hidden: function()...
that has probably been there for a while:
return id.substring( id, id.length - 5 );
should be
return id.substring( 0, id.length - 5 );
[34991] removed the IDs from the "columns" checkboxes :( but they're needed for getting checkboxes to be saved via AJAX. Not sure about the "Boxes" checkboxes, still investigating.