Bug Tracker

Opened 7 years ago

Closed 7 years ago

#12813 closed bug (notabug)

jQuery.clean does not collect scripts from document fragments

Reported by: iamnoah (@… Owned by: iamnoah (@…
Priority: low Milestone: 1.9
Component: manipulation Version: 1.8.2
Keywords: Cc:
Blocked by: Blocking:

Description

Chrome 22.0.1229.94 on Mac OS 10.7.4 (but probably all browser/OS)

Demo: http://jsfiddle.net/XsZLt/

When using any DOM manipulation method with a document fragment, script tags inside the fragment are not executed as they would be if the argument was the fragment's HTML as a string.

Expected Behavior: Script tags are executed by dom manipulation methods, regardless of where the originate from.

This is desirable because many frameworks use document fragments to pre-process elements before insertion.

Change History (11)

comment:1 Changed 7 years ago by Rick Waldron

Repost:

The test case given in the ticket is using jQuery.buildFragment, which is not a publicly supported API

comment:2 Changed 7 years ago by dmethvin

I really dislike the sneaky nature of injecting script tags as a way of immediately executing script. We don't document that it works. Can you show some use cases?

comment:3 Changed 7 years ago by Rick Waldron

I dislike the idea of adding a new call signature overload for html()...

comment:4 Changed 7 years ago by anonymous

The demo isn't a real use case, but that's what it simplifies down to.

My use case is loading HTML content. Our framework does some preprocessing before inserting the content and the result is a document fragment. It's legacy content, so it has inline scripts that set up the content.

I'm surprised to hear that DocumentFragments are not officially supported, because I've seen lots of jQuery client code passing them in. Internally, it's all document fragments anyway, right?

comment:5 Changed 7 years ago by dmethvin

Owner: set to iamnoah (@…
Status: newpending

At this point I'm just trying to understand the use case. For example,

Expected Behavior: Script tags are executed by dom manipulation methods, regardless of where the originate from.

Actually, the test case not only expects the scripts to be executed, but that they will be synchronous. Otherwise the unit test would fail. Is there the same expectation for external scripts?

And I'm still not sure how the document fragments arrive at .html() from a framework. @iamnoah can you provide some links to examples?

comment:6 in reply to:  5 Changed 7 years ago by iamnoah

Replying to dmethvin:

At this point I'm just trying to understand the use case. For example,

Expected Behavior: Script tags are executed by dom manipulation methods, regardless of where the originate from.

Actually, the test case not only expects the scripts to be executed, but that they will be synchronous. Otherwise the unit test would fail. Is there the same expectation for external scripts?

For my use case it doesn't matter. I assumed sync in the test case to keep it simple and since that's consistent with the behavior for a string fragment.

And I'm still not sure how the document fragments arrive at .html() from a framework. @iamnoah can you provide some links to examples?

Here's an example: https://github.com/jupiterjs/canjs/blob/master/view/modifiers/modifiers.js#L65

It uses some of the private API, but the use case is valid: applying template hookups before the content is inserted. You could do the same thing without using any jQuery internals but still want to insert a document fragment.

comment:7 Changed 7 years ago by dmethvin

Component: unfiledmanipulation
Milestone: None1.9
Owner: changed from iamnoah (@… to gibson042
Priority: undecidedlow
Status: pendingassigned

The reason I'm asking is because we're probably getting rid of .buildFragment entirely in 2.0 (see #10515); we will need to break it in 1.9 as well to ensure that nobody gets all up in arms that there is an API difference between the two jQuery versions when we said there would be none.

The specific issue of working in a fragment should probably be tackled as part of #11795 since it needs to be tested thoroughly and we'll be actually documenting the supported behavior resulting from that. @gibson042 does that make sense?

comment:8 Changed 7 years ago by gibson042

Owner: changed from gibson042 to iamnoah (@…
Status: assignedpending

I'm not at all opposed to supporting document fragments, but it'd have to be in the whole suite of manipulation methods, which means that the bulk of such work would be in adding unit tests.

@iamnoah, the source change in your PR seems reasonable, and I invite you to submit a new one (with much more thorough testing) after https://github.com/jquery/jquery/pull/864 is resolved. You can get started on the latter part now, and hit me up on freenode IRC in #jquery-dev if you have any questions about what we're looking for.

comment:9 Changed 7 years ago by gibson042

Status: pendingopen

comment:10 Changed 7 years ago by dmethvin

Okay, now that $.buildFragment() is no more it would be good to have a test case using a documented interface that we could use to fix this.

comment:11 Changed 7 years ago by dmethvin

Resolution: notabug
Status: openclosed

I'm going to close this since it was using an undocumented interface.

Note: See TracTickets for help on using tickets.