WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#42506 closed defect (bug) (fixed)

Customize: Custom Link URL validation for nav menu items doesn't allow jump links

Reported by: ravipatel Owned by: westonruter
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Menus Keywords: has-patch needs-testing needs-unit-tests dev-reviewed
Focuses: javascript Cc:

Description

Twenty Seventeen Menu Customization Link validation Need to allowed with # same like menu management.

Attachments (6)

link_validation_menu.png (22.8 KB) - added by ravipatel 8 months ago.
link_validation_menu.2.png (22.8 KB) - added by ravipatel 8 months ago.
menu_customize.png (50.7 KB) - added by ravipatel 8 months ago.
42506.diff (634 bytes) - added by westonruter 8 months ago.
42506.2.diff (2.0 KB) - added by westonruter 8 months ago.
42506.3.diff (2.0 KB) - added by westonruter 8 months ago.

Download all attachments as: .zip

Change History (19)

This ticket was mentioned in Slack in #core-customize by presskopp. View the logs.


8 months ago

#2 @westonruter
8 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.9
  • Summary changed from Twenty Seventeen Menu Customization Link validation to Customize: Custom Link URL validation for nav menu items doesn't allow jump links

Good catch. This issue was introduced in #32816 via [41697]. We need to allow any URL that begins with #.

@westonruter
8 months ago

#3 @westonruter
8 months ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Owner set to westonruter
  • Status changed from new to accepted

Please test 42506.diff.

#4 @johnbillion
8 months ago

  • Keywords needs-refresh needs-unit-tests added

This is also blocking URLs that contain domains without periods, for example http://localhost.

@westonruter
8 months ago

#5 @westonruter
8 months ago

  • Keywords needs-refresh removed

Clearly the regular expression we incorporated was not robust enough, while also being overly complicated. In 42506.2.diff I've replaced it with a much simpler set of checks to confirm it roughly looks like a URL. Any further validation will then be handled on the server by WP_Customize_Nav_Menu_Item_Setting::sanitize() and esc_url_raw().

#6 @westonruter
8 months ago

@ravipatel Would you test 42506.2.diff?

This ticket was mentioned in Slack in #core-committers by westonruter. View the logs.


8 months ago

#8 @SergeyBiryukov
8 months ago

  • Keywords dev-reviewed added

Tested 42506.2.diff, works as expected.

#9 @SergeyBiryukov
8 months ago

@westonruter The new regex allows http:// though, which was previously disallowed. I guess it should still be disallowed, as it's not a valid URL.

Last edited 8 months ago by SergeyBiryukov (previous) (diff)

@westonruter
8 months ago

#10 @westonruter
8 months ago

@SergeyBiryukov good catch. 42506.3.diff fixes that issue, and also prevents https:// from being allowed.

#12 @westonruter
8 months ago

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

In 42153:

Customize: Restore ability to add Custom Link nav menu items for jump links and other URLs that were previously allowed.

Simplify regular expression for checking URL validity to just do basic checks to confirm the value looks like a URL. Leave the complete validation to the server-side logic in WP_Customize_Nav_Menu_Item_Setting::sanitize() to avoid having to maintain two separate codebases for validating URLs.

Props westonruter, SergeyBiryukov for testing.
Amends [41697].
See #32816.
Fixes #42506 for trunk.

#13 @westonruter
8 months ago

In 42154:

Customize: Restore ability to add Custom Link nav menu items for jump links and other URLs that were previously allowed.

Simplify regular expression for checking URL validity to just do basic checks to confirm the value looks like a URL. Leave the complete validation to the server-side logic in WP_Customize_Nav_Menu_Item_Setting::sanitize() to avoid having to maintain two separate codebases for validating URLs.

Props westonruter, SergeyBiryukov for testing.
Amends [41697].
See #32816.
Fixes #42506 for 4.9.

Note: See TracTickets for help on using tickets.