WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 21 months ago

#21448 new defect (bug)

unfiltered HTML comments from a frame js fix breaks js in windowed comments

Reported by: nerrad Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.4.1
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

Noticed this today.

We have comments loading up in a jquery colorbox on a site being developed and it loads the comment form via ajax. When it's loaded, the fix in #20812 breaks javascript because the property "name" is null.

Attaching a patch after posting this for only executing when document.getElementById("_wp_unfiltered_html_comment_disabled") != null.

Attachments (2)

21448.patch (821 bytes) - added by nerrad 21 months ago.
fix for name property = null
rdp-comment-test.zip (108.4 KB) - added by nerrad 21 months ago.
simple plugin for replicating bug

Download all attachments as: .zip

Change History (9)

nerrad21 months ago

fix for name property = null

comment:1 nacin21 months ago

  • Keywords reporter-feedback added

I think the name property might be null because it isn't finding the element. But this element is printed *right* before the <script> tags. Could you provide more steps to reproduce?

comment:2 nerrad21 months ago

Yeah, I was surprised too. I'll try and whip up an example that reproduces the error today.

nerrad21 months ago

simple plugin for replicating bug

comment:3 nerrad21 months ago

attached is a simple plugin that will help replicate this.

The plugin:

  • loads colorbox
  • hooks into the_content to add a test link for initializing colorbox and the test in a post (view on single.php).
  • has some javascript for doing an ajax call that returns the contents of do_action('comment_form'); which is what we're testing.

Just use this plugin on the twenty-eleven theme (for testing purposes) and comment out <?php comments_template( '', true ); ?> in the single.php file of twentyeleven.

To replicate after setting up the above.

  • while viewing single.php view of a post click on the "Click here for comment popup" link and javascript fails because of the null value.

The plugin is just a quick and dirty mockup to replicate the bug.

comment:4 nerrad21 months ago

  • Keywords reporter-feedback removed

comment:5 SergeyBiryukov21 months ago

The problem is that ColorBox uses append() to add the response to DOM:
https://github.com/jackmoore/colorbox/blob/master/colorbox/jquery.colorbox.js#L592

According to Karl Swedberg's comment:

All of jQuery's insertion methods use a domManip function internally to clean/process elements before and after they are inserted into the DOM. One of the things the domManip function does is pull out any script elements about to be inserted and run them through an "evalScript routine" rather than inject them with the rest of the DOM fragment. It inserts the scripts separately, evaluates them, and then removes them from the DOM.

So with the attached plugin active, the script in wp_comment_form_unfiltered_html_nonce() is executed earlier than the corresponding input element is inserted into DOM.

comment:6 SergeyBiryukov21 months ago

A workaround for the plugin would be to remove the script from the response and re-add it using document.createElement() after ColorBox is initialized, as suggested in the related thread on Stack Overflow.

comment:7 nerrad21 months ago

Well the workaround I'm doing currently is just removing the existing action and adding in my own action. The comments you made here Sergey make sense but still think it's worth looking at how the <script> is implemented here as there may be more scenarios (albeit fringe) where this error may pop up.

Maybe instead of what I put in the patch, there should just be some error output when the name property is "null" (rather than stopping javascript all together)?

Note: See TracTickets for help on using tickets.