Incorrect usage of esc_attr() for URL escaping

[hardik2221]

File: src/wp-admin/includes/class-walker-nav-menu-checklist.php
URL is being escaped using esc_attr() instead of esc_url()

[nareshbheda]

Same issue found in another file: src/wp-admin/includes/class-walker-nav-menu-edit.php

[nareshbheda]

Patch

[dilipbheda]

Test Report

Description

I tested both patches, and they seem to be working well with the following test scenarios:

1. A valid HTTP URL
2. A # symbol and an empty string
3. A #target_element_id

Patch tested:
1) https://core.trac.wordpress.org/attachment/ticket/63351/63351.2.patch
2) https://core.trac.wordpress.org/attachment/ticket/63351/63351.patch

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.27
• Server: nginx/1.27.3
• Database: mysqli (Server: 8.0.40 / Client: mysqlnd 8.2.27)
• Browser: Chrome 135.0.0.0
• OS: Linux
• Theme: Twenty Twenty-One 2.5
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅ Issue resolved with patch.

[SirLouen]

@hardik2221 @nareshbheda just in case you like to keep sorting this kind of PHPCS issues, its a good thing to know that for these kind of tasks that doesn't provide a big change in the code, there are what they call "Blessed tasks". Every new cycle they open a new task for many things like these.

For 6.9 this is the task for PHPCS: #63168

Once you submit the patch you can post there to inform, and they will be reviewing and committing the new changes

@dilipbheda no big need to test PHPCS patches. They are too trivial. You can test them if you like, but there are many other patches that need priority testing (See for example: #63151 #63316 #63135). If you want to know more about priority testing patches, ping me at slack (same username)

[dilipbheda]

@dilipbheda no big need to test PHPCS patches. They are too trivial. You can test them if you like, but there are many other patches that need priority testing (See for example: #63151 #63316 #63135). If you want to know more about priority testing patches, ping me at slack (same username)

@SirLouen Thanks for sharing the priority tickets. I’ll take a look at them.

[SergeyBiryukov]

In 60213:

Coding Standards: Use correct escaping function for nav menu item URLs.

Follow-up to [14248], [15077].

Props hardik2221, nareshbheda, dilipbheda, SirLouen.
Fixes #63351.

[dilip2615]

Patch Testing Report

Patch tested: 63351.diff (uses esc_url() with correct $menu_item->url)

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.29.1
• Database: mysqli (Server: 8.4.6 / Client: mysqlnd 8.2.28)
• Browser: Chrome 140.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty 2.9
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Steps
1) Replaced esc_attr() with esc_url() and corrected variable from $menu_item->url in class-walker-nav-menu-edit.php.
2) Appearance → Menus → Custom Links: tested

• ​https://example.com/?a=1&b=2
• ​http://example.com/über
• javascript:alert(1)

3) Verified input value via DevTools.

Results

• Valid/UTF-8 URLs render correctly in the value attribute (URL-escaped, no double-escape).
• Invalid scheme sanitized on save.
• PHPUnit default suite: OK.

Conclusion
✅ Patch behaves as expected. (URL-specific escaping + correct variable name)