WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#23681 closed defect (bug) (fixed)

jQuery 1.9 doesn't like leading whitespace from wp_ajax_add_menu_item

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

Description

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 5 years ago.
Call $.parseHTML instead of $()
23681.patch (1.1 KB) - added by ocean90 4 years ago.
23681.2.patch (1.0 KB) - added by ocean90 4 years ago.
With $.trim()
23681.3.patch (1.9 KB) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (15)

@csixty4
5 years ago

Call $.parseHTML instead of $()

#1 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 3.6

#2 @SergeyBiryukov
5 years ago

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

#3 @ocean90
4 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.

http://blog.jquery.com/2013/05/24/jquery-1-10-0-and-2-0-1-released/

@ocean90
4 years ago

#4 @ocean90
4 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
4 years ago

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

#6 follow-up: @SergeyBiryukov
4 years ago

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

#7 in reply to: ↑ 6 @ocean90
4 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.

@ocean90
4 years ago

With $.trim()

#8 @nacin
4 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
4 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#10 @nacin
4 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.

@ocean90
4 years ago

#11 @ocean90
4 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.