Bug Tracker

#15209 closed bug (patchwelcome)

Opened August 10, 2014 05:07PM UTC

Closed August 10, 2014 10:16PM UTC

Last modified August 12, 2014 12:53AM UTC

Scripts not being evaluated

Reported by: Dasher225 Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 1.11.1
Keywords: Cc:
Blocked by: Blocking:
Description

Scripts are not being evaluated when I expect them to.

See here: http://jsfiddle.net/fz53r1kj/1/

In the jsFiddle, str1 is equal to the following. str2 is the same without the first script (which sorts).

<script>

var i, len, sortedChildren;

sortedChildren = $('.rows').children().sort(function (a, b) {

return $(a).data('rank') < $(b).data('rank') ? -1 : 1;

});

for (i = 0, len = sortedChildren.length; i < len; i++) {

child = sortedChildren[i];

$('.rows').append(child);

}

</script>

<div class="rows">

<div data-rank="3">

Three

<script>$(‘[data-rank=3]’).css(color: ‘red’);</script>

</div>

<div data-rank="2">

Two

<script>$(‘[data-rank=2]’).css(color: ‘green’);</script>

</div>

<div data-rank="1">

One

<script>$(‘[data-rank=1]’).css(color: ‘blue’);</script>

</div>

</div>

When appending str1, the scripts in the rows do not run.

When appending str2, the scripts in the rows run.

Thus it appears appending the elements a second time prevents the scripts inside from running.

Using jsFiddle this appears in chrome, firefox, and safari on Mac for all jquery versions 1.9.1 and later. It works as expected in 1.8.3 and earlier.

Attachments (0)
Change History (4)

Changed August 10, 2014 10:16PM UTC by gibson042 comment:1

resolution: → patchwelcome
status: new → closed

I tried to clean up your fiddle for better readability: http://jsfiddle.net/fz53r1kj/2/

And it looks like you're really playing with fire. After your HTML (<script>…</script><div id="rows">…</div>) enters the body, we attempt to evaluate all four scripts (one doing $rows.append( $rows.children().sort() ) and three doing $row.css( "color", … )). However, the first script manipulates containing elements of the later scripts, and when that happens our buildFragment code backing .append finds them in the document and therefore marks them as ''already'' executed, causing them to be skipped after the first script finishes. In short, recursive DOM manipulation doesn't play nice with synchronous (and singular) script execution.

I don't want to spend any time on this hairy edge case, but if you do then I would consider landing a solid (and small) pull request. However, that time would probably be better spent evaluating other ways to solve whatever problem led you to this bug report—if you can factor out the dependence on jQuery evaluating scripts for you, you will be happier when our behavior changes in future releases.

Changed August 11, 2014 05:10PM UTC by Dasher225 comment:2

Yep I learned what was causing them when trying to figure out what was wrong.

I do believe this is an edge case and I worked around it by just doing the sorting later.

One thing I don't understand though is why scripts are marked as run when they are appended after already being in the dom instead of when they are actually run.

Would changing where scripts are marked as run be an acceptable solution?

Changed August 11, 2014 06:43PM UTC by gibson042 comment:3

Replying to [comment:2 Dasher225]:

One thing I don't understand though is why scripts are marked as run when they are appended after already being in the dom instead of when they are actually run. Would changing where scripts are marked as run be an acceptable solution?

Only if it covered the case of scripts in the document that were not put there by jQuery (e.g., <div id="foo"><script>runOnce()</script></div><script>$("#foo").appendTo("#newContainer")</script>). This is why any script we find in the document is assumed to have already executed (and note that we mark scripts as executed when we ''find'' them instead of when we execute them because many scripts—in fact most—are executed outside of our knowledge and control).

Changed August 12, 2014 12:53AM UTC by Dasher225 comment:4

Auhh cool. Thank you for the context. I will try and come up with a solution that takes that into account