WordPress.org

Make WordPress Core

Opened 14 months ago

Closed 3 months ago

Last modified 3 months ago

#48011 closed defect (bug) (fixed)

Having issue with navigation menu not updated

Reported by: zaheerahmad Owned by: SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-screenshots has-patch has-unit-tests commit
Focuses: Cc:

Description

Problem:
for example my category name “soup, noodle & pasta” I add this category in my menu and after that will change my category name “pizza, pasta & noddles” it should automatically update in in menu too.

but unfortunately it's not working.

Think I already did I tried to disabled all plugin not happened I tried to change theme same result

Test case If you have category simple category name e.g "contact" it will work fine.

My issue is when I put special character like "," or "&" i face this issue

Attachments (6)

trac-48011.diff (487 bytes) - added by achyuthajoy 14 months ago.
Patch to fix problems with special characters & quotes on categories while adding them to menus
48011-1.patch (1.6 KB) - added by achyuthajoy 13 months ago.
48011.2.diff (1.8 KB) - added by donmhico 10 months ago.
Included unit tests and use strict comparison.
48011-3.diff (2.0 KB) - added by birgire 8 months ago.
48011-4.diff (2.1 KB) - added by birgire 8 months ago.
48011.5.diff (2.1 KB) - added by audrasjb 4 months ago.
Patch refreshed for WP 5.5

Download all attachments as: .zip

Change History (30)

#1 follow-up: @achyuthajoy
14 months ago

  • Keywords needs-screenshots reporter-feedback added; needs-patch removed

Hi @zaheerahmad!

I've tried the steps you've mentioned and couldn't reproduce the problem. I've also tried different combinations with special characters and the menu updated as expected.

Try switching your theme to Twenty Nineteen, turn of all plugins and see if you can reproduce the problem. If it appears on Twenty Nineteen, please share a screenshot / screen cast.

#2 in reply to: ↑ 1 @zaheerahmad
14 months ago

@achyuthajoy I test on fresh installed wordpress and yes theme is Twenty Nineteen and I deactivate all plugins I also create the video you can see in video

again I repeat my issue when I create one menu with categories after that when I update my category name, In menu category name not update automatically if i use any special character e.g ", or &" but if i change simple category name like hello it will be change automatically.

you can see all this thing in above video link

regards

#3 @zaheerahmad
14 months ago

  • Keywords needs-patch added

#4 @achyuthajoy
14 months ago

  • Keywords has-screenshots added; needs-screenshots reporter-feedback removed

Hey @zaheerahmad Thanks for the video. I was able to replicate the problem you've mentioned using the Navigation widget.

Found that when a category is created, the inputs are encoded (https://developer.wordpress.org/reference/functions/_wp_specialchars/). However, when they're read from the db for using in the menu, it's not decoded making the Category Title act as a Custom label.

I've made a patch to fix it and will update the thread shortly.

@achyuthajoy
14 months ago

Patch to fix problems with special characters & quotes on categories while adding them to menus

#5 @achyuthajoy
14 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

This ticket was mentioned in Slack in #core by achyuthajoy. View the logs.


14 months ago

#7 @achyuthajoy
14 months ago

Here are the steps to re-produce the problem.

  1. Add a Post Category with either quotes or special characters like &, > or <.
  2. Create a new menu and add the category to it.
  3. Create a footer navigation widget and choose the created menu.
  4. Edit & Change the category name under Posts -> Categories to notice that the change didn't appear in the footer.
  5. Try creating a new Category without any quotes or special characters and add it to the menu. Try changing the category name to something else to see that the changes are reflected in the footer.

P.S - This bug only when a category is created for the first time and it's name contains quotes or special characters. If an existing category is added to the menu and renamed to have quotes or special characters, doesn't cause this bug.

#8 follow-up: @desrosj
13 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from major to normal

Thanks for this ticket, @zaheerahmad, and thanks for investigating, @achyuthajoy!

I am able to reproduce the issue described with the steps that were outlined.

However, it seems that trac-48011.diff is not applying for me. Looking at src/wp-includes/nav-menu.php (source), I actually don't see the line being replaced in the patch, and the suggested change is already present. Can you take another look @zaheerahmad? Some unit tests showing the problem may also be helpful.

I am going to put this in the Future Release milestone. But, once a patch is ready and tested, this can be moved into a numbered milestone.

#9 in reply to: ↑ 8 @achyuthajoy
13 months ago

  • Keywords has-patch added; needs-patch removed

Thanks for taking a look @desrosj. It seems the diff file I added was incorrect. It changes should be in reverse. I'm adding a new patch file.

Thanks for this ticket, @zaheerahmad, and thanks for investigating, @achyuthajoy!

I am able to reproduce the issue described with the steps that were outlined.

However, it seems that trac-48011.diff is not applying for me. Looking at src/wp-includes/nav-menu.php (source), I actually don't see the line being replaced in the patch, and the suggested change is already present. Can you take another look @zaheerahmad? Some unit tests showing the problem may also be helpful.

I am going to put this in the Future Release milestone. But, once a patch is ready and tested, this can be moved into a numbered milestone.

#10 @pento
13 months ago

  • Keywords needs-unit-tests added

Patch looks good, could you add some unit tests?

tests/phpunit/tests/post/nav-menu.php would be a good place to add the tests.

#11 @SergeyBiryukov
12 months ago

  • Milestone changed from Future Release to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

@donmhico
10 months ago

Included unit tests and use strict comparison.

#12 @donmhico
10 months ago

  • Keywords needs-unit-tests removed

#13 @audrasjb
10 months ago

  • Keywords needs-testing removed
  • Version 5.2.3 deleted

Hi,

Thanks @donmhico for the unit tests. I tested the above patch and it looks good to me, the patch is fixing the issue as expected.
Let's milestone that for 5.4.

#14 @audrasjb
8 months ago

  • Keywords commit added

@birgire
8 months ago

#15 @birgire
8 months ago

  • Keywords has-unit-tests added

48011-3.diff refreshes 48011.2.diff that didn't apply clean on my install.

It further:

  • Preserves the existing == comparison instead of ===. Just being cautios of changing those, as in general such changes can bite :-) Changing cat names through the UI requires a name, so the strict comparison is most likely ok :-)
  • Expands the test name string to 'Test Cat - \"Pre-Slashed\" Cat Name & >' to cover the pre-slashed case.
  • Replaces array alias [] for array() in the test.
  • Adjustments according to the coding standard.
  • Adjusts the inline docs of the test.
Last edited 8 months ago by birgire (previous) (diff)

@birgire
8 months ago

#16 @birgire
8 months ago

48011-4.diff just removes my debug assignment relic and has a variation of the inline array comment in the test.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 months ago

#18 @mikeschroder
8 months ago

Just a quick note that I tested 48011-4.diff and it looks to be working well + test passing.

#19 @SergeyBiryukov
8 months ago

  • Milestone changed from 5.4 to 5.5

Would like to give this a closer look, moving to 5.5 for now.

#20 @audrasjb
4 months ago

  • Keywords commit removed

@audrasjb
4 months ago

Patch refreshed for WP 5.5

#21 follow-up: @audrasjb
4 months ago

  • Keywords commit added

Patch still works fine. I only took care of refreshing the current patch so it could hopefully ship with 5.5.

#22 in reply to: ↑ 21 @achyuthajoy
4 months ago

Replying to audrasjb:

Patch still works fine. I only took care of refreshing the current patch so it could hopefully ship with 5.5.

Thanks @audrasjb :+1

#23 @whyisjake
3 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 48416:

Menus: Ensure that category menus with special characters can be updated.

Slashes and HTML encoding could cause some menus not to be updated.

Fixes #48011.

Props zaheerahmad, achyuthajoy, desrosj, pento, SergeyBiryukov, donmhico, audrasjb, birgire, mikeschroder.

#24 @SergeyBiryukov
3 months ago

In 48439:

Menus: Simplify the test for wp_update_nav_menu_item() with special characters in category name.

The menu-item-title value is saved as a post_title property, so the resulting property can be checked directly, without a callback.

Follow-up to [48416].

See #48011.

Note: See TracTickets for help on using tickets.