Bug Tracker

Ticket #11795 (closed feature: fixed)

Opened 3 years ago

Last modified 21 months ago

Resolve script manipulation/execution inconsistencies

Reported by: gibson042 Owned by: gibson042
Priority: high Milestone: 1.9
Component: manipulation Version: 1.7.2
Keywords: 1.9-discuss Cc:
Blocking: #9134, #10470 Blocked by:

Description

.domManip's treatment of <script>s is undocumented, inconsistent, and often counterintuitive. We should design a sane approach and document the hell out of it in all derived methods.

 http://jsfiddle.net/jR3H6/1/

Special attention should be given to the following:

  • Breaking old code
  • Performance
  • Iteration (e.g., $("<div><a/><b/></div>").children().append("<script/>"))

Change History

comment:1 Changed 3 years ago by gibson042

To get discussion started, I think we should never automatically detach elements. As for execution, I'd like to investigate the possibility of skipping it for scripts that already in the DOM before manipulation and just moving around.

comment:2 Changed 3 years ago by dmethvin

I agree with this. See #3105 for some history though. I consider the liberal acceptance of <script> tags to be a security hole, it would have been nicer if we had a way for the user to explicitly say they wanted to run any scripts.

comment:3 Changed 3 years ago by gibson042

Issues like that are foremost in my mind. It's necessary to do something with scripts before dropping them in the DOM so we can prevent certain misbehaving browsers from executing them automatically, and it's also necessary to provide continuity with the past decision to execute them "once" as they land, but beyond that the landscape of possibilities is fairly open. The most important thing is having a productive discussion now to agree on and document a policy, hopefully one that will allow us to take further positive steps in the future.

Assuming that we'll still be running scripts through globalEval, and that we don't want to accidentally do so too often, I think we can meet the spirit of previous versions by assuming that anything already in the document has already been executed (and that there's no reason to execute anything until it actually hits the document). And if we're okay with re-executing .detached scripts, we can accomplish it without any new _data or .clone issues or expensive code outside of domManip. Of course, "once and only once" is also possible if that's what we want.

To ease the transition, we might want to expose a convenience method that makes it easy to scrub a jQuery set... maybe a generalization of the input/script loops in jQuery.clean. Longer term, a flag controlling script execution is also a possibility.

comment:4 Changed 3 years ago by dmethvin

#11878 is a duplicate of this ticket.

comment:5 Changed 3 years ago by dmethvin

Once we decide what to do here, we need to document it. Our handling of scripts is undocumented, which is good since it's not a point of pride. :)

comment:6 Changed 2 years ago by dmethvin

  • Status changed from new to open
  • Component changed from unfiled to manipulation

comment:7 Changed 2 years ago by gibson042

  • Keywords 1.9-discuss added

comment:8 Changed 2 years ago by dmethvin

  • Type changed from enhancement to feature

Bulk change from enhancement to feature.

comment:9 Changed 2 years ago by dmethvin

I really need to see a plan of attack here before voting.

comment:10 Changed 2 years ago by gibson042

 https://github.com/jquery/jquery/pull/864 is the working proof of concept, in which we mark in-document scripts as executed on detach, clone, clean, and domManip. It has a lot of oldIE code that I'll be happy to see go in 2.0.

comment:11 follow-up: ↓ 13 Changed 2 years ago by mikesherov

this ticket seems like a "god ticket". for example, does #10470 fall under this?

comment:12 Changed 2 years ago by mikesherov

+1, All for making this consistent, no matter the specifics.

comment:13 in reply to: ↑ 11 Changed 2 years ago by gibson042

Replying to mikesherov:

this ticket seems like a "god ticket". for example, does #10470 fall under this?

I would say yes, even if the draft patch doesn't yet resolve it.

comment:14 Changed 2 years ago by timmywil

  • Priority changed from undecided to high

comment:15 Changed 2 years ago by mikesherov

  • Blocking 10470 added

comment:16 Changed 2 years ago by mikesherov

  • Blocking 9134 added

comment:17 Changed 2 years ago by gibson042

+1

comment:18 Changed 2 years ago by mikesherov

  • Milestone changed from None to 1.9

comment:19 Changed 2 years ago by gibson042

  • Owner set to gibson042
  • Status changed from open to assigned

comment:20 Changed 2 years ago by Richard Gibson

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

Fix #11795, #10470: keep scripts in DOM; execute only on first insertion. Close gh-864.

Changeset: e889134058232c5e19156353c5fc3bf3b4915a94

comment:13 Changed 21 months ago by mikesherov

#13634 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.