WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 5 weeks ago

#48011 reviewing defect (bug)

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 commit has-unit-tests
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 (5)

trac-48011.diff (487 bytes) - added by achyuthajoy 7 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 7 months ago.
48011.2.diff (1.8 KB) - added by donmhico 3 months ago.
Included unit tests and use strict comparison.
48011-3.diff (2.0 KB) - added by birgire 6 weeks ago.
48011-4.diff (2.1 KB) - added by birgire 6 weeks ago.

Download all attachments as: .zip

Change History (24)

#1 follow-up: @achyuthajoy
7 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
7 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
7 months ago

  • Keywords needs-patch added

#4 @achyuthajoy
7 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
7 months ago

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

#5 @achyuthajoy
7 months ago

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

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


7 months ago

#7 @achyuthajoy
7 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
7 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
7 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
6 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
5 months ago

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

@donmhico
3 months ago

Included unit tests and use strict comparison.

#12 @donmhico
3 months ago

  • Keywords needs-unit-tests removed

#13 @audrasjb
3 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
6 weeks ago

  • Keywords commit added

@birgire
6 weeks ago

#15 @birgire
6 weeks 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 6 weeks ago by birgire (previous) (diff)

@birgire
6 weeks ago

#16 @birgire
6 weeks 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.


5 weeks ago

#18 @mikeschroder
5 weeks ago

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

#19 @SergeyBiryukov
5 weeks ago

  • Milestone changed from 5.4 to 5.5

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

Note: See TracTickets for help on using tickets.