Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#23681 closed defect (bug) (fixed)

jQuery 1.9 doesn't like leading whitespace from wp_ajax_add_menu_item

Reported by: csixty4's profile csixty4 Owned by: ocean90's profile ocean90
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.6
Component: Menus Keywords: has-patch 3.7-early
Focuses: Cc:


jquery-migrate logs this message to the console when adding a new menu item:

JQMIGRATE: $(html) HTML strings must start with '<' character

Walker_Nav_Menu_Edit returns a <li> indented with two tabs because there's inline HTML in that class and it's indented. jQuery 1.9 rejects HTML strings with leading whitespace for security reasons.

(Note: this is probably why jquery-migrate is calling parseHTML instead of using a regex in #23055)

This patch uses $.parseHTML on the HTML fragment instead of passing it directly to $(). As it's written, $.parseHTML will strip out any scripts in the fragment for security reasons. If you think a plugin might include a script for some reason, the call should be changed to $.parseHTML(menuMarkup, document, true).

Attachments (4)

nav-menu_0.patch (516 bytes) - added by csixty4 10 years ago.
Call $.parseHTML instead of $()
23681.patch (1.1 KB) - added by ocean90 10 years ago.
23681.2.patch (1.0 KB) - added by ocean90 10 years ago.
With $.trim()
23681.3.patch (1.9 KB) - added by ocean90 10 years ago.

Download all attachments as: .zip

Change History (15)

10 years ago

Call $.parseHTML instead of $()

#1 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 3.6

#2 @SergeyBiryukov
10 years ago

Custom fields are also affected: ticket:22975:17.

#3 @ocean90
10 years ago

jQuery 1.10.0 (#24426) has relaxed the HTML parsing, means it supports a leading whitespace again. But:

We still strongly advise that you use $.parseHTML() when parsing HTML obtained from external sources, and may be making further changes to HTML parsing in the future.

10 years ago

#4 @ocean90
10 years ago

23681.patch calls $.parseHTML on custom fields too. Here we get an array with two DOM nodes, the first one is an empty line, means text. So we have to pass the second index to jQuery.

#5 @SergeyBiryukov
10 years ago

Perhaps $.trim() would be faster (suggested in ticket:22975:21)?

#6 follow-up: @SergeyBiryukov
10 years ago

FWIW, custom fields work fine in trunk, see ticket:22975:30.

#7 in reply to: ↑ 6 @ocean90
10 years ago

Replying to SergeyBiryukov:

FWIW, custom fields work fine in trunk, see ticket:22975:30.

Yes, but you don't know how long, see comment:3. $.trim() is a good idea. Testing.

10 years ago

With $.trim()

#8 @nacin
10 years ago

  • Keywords 3.7-early added
  • Milestone changed from 3.6 to Future Release

This good to go? We can always just wait until 3.7, as 3.6.x will be shipping with 1.10.x.

#9 @wonderboymusic
10 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#10 @nacin
10 years ago

  • Owner set to ocean90
  • Status changed from new to reviewing

ocean90, please commit this if it should be, or close this if it should not.

10 years ago

#11 @ocean90
10 years ago

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

In 25546:

Trim leading whitespace from AJAX responses.

This fixes the warning "$(html) HTML strings must start with '<' character" by jQuery Migrate when adding nav menu items, post custom fields or comment replies.

fixes #23681.

Note: See TracTickets for help on using tickets.