![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#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 |
|---|---|---|---|
| Priority: | normal | Milestone: | 4.9 |
| Component: | Menus | Version: | 4.9 |
| Severity: | normal | Keywords: | has-patch needs-testing needs-unit-tests dev-reviewed |
| Cc: | Focuses: | javascript |
Description
Twenty Seventeen Menu Customization Link validation Need to allowed with # same like menu management.
Attachments (6)
Change History (19)
This ticket was mentioned in Slack in #core-customize by presskopp. View the logs.
9 years ago
#2
@
9 years ago
- Keywords needs-patch added
- Milestone Awaiting Review → 4.9
- Summary Twenty Seventeen Menu Customization Link validation → Customize: Custom Link URL validation for nav menu items doesn't allow jump links
#3
@
9 years ago
- Keywords has-patch needs-testing added; needs-patch removed
- Owner set to
- Status new → accepted
Please test 42506.diff.
#4
@
9 years ago
- Keywords needs-refresh needs-unit-tests added
This is also blocking URLs that contain domains without periods, for example http://localhost.
#5
@
9 years 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
@
9 years ago
@ravipatel Would you test 42506.2.diff?
This ticket was mentioned in Slack in #core-committers by westonruter. View the logs.
9 years ago
#9
@
9 years 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.
#10
@
9 years ago
@SergeyBiryukov good catch. 42506.3.diff fixes that issue, and also prevents https:// from being allowed.
#11
@
9 years ago
42506.3.diff looks good to me.
Good catch. This issue was introduced in #32816 via [41697]. We need to allow any URL that begins with
#.