Skip to content

Commit

Permalink
Element#update now takes care of SCRIPT elements in IE. [#573 state…
Browse files Browse the repository at this point in the history
…:resolved]
  • Loading branch information
Juriy Zaytsev committed Mar 22, 2009
1 parent 2c986d8 commit 71a8663
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG
@@ -1,3 +1,5 @@
* `Element#update` now takes care of SCRIPT elements in IE. [#573 state:resolved] (Martin, Tobie Langel, kangax)

* Remove unused local variables from `Element.extend`. Fix one of the form tests to remove `_extendedByPrototype` by setting it to `undefined` rather than `false` (`_extendedByPrototype` being `false` does not force `Element.extend` to re-extend element). (T.J. Crowder, kangax)

* Make test for `escapeHTML`/`unescapeHTML` more strict. (Chrome 1.x escapes "<" and "&" with `innerHTML`, but not ">") (kangax)
Expand Down
23 changes: 22 additions & 1 deletion src/dom/dom.js
Expand Up @@ -203,6 +203,20 @@ Element.Methods = {
}
})();

var SCRIPT_ELEMENT_REJECTS_TEXTNODE_APPENDING = (function () {
var s = document.createElement("script"),
isBuggy = false;
try {
s.appendChild(document.createTextNode(""));
isBuggy = !s.firstChild ||
s.firstChild && s.firstChild.nodeType !== 3;
} catch (e) {
isBuggy = true;
}
s = null;
return isBuggy;
})();

function update(element, content) {
element = $(element);

Expand All @@ -213,9 +227,16 @@ Element.Methods = {
return element.update().insert(content);

content = Object.toHTML(content);

var tagName = element.tagName.toUpperCase();

if (tagName === 'SCRIPT' && SCRIPT_ELEMENT_REJECTS_TEXTNODE_APPENDING) {
// scripts are not evaluated when updating SCRIPT element
element.text = content;
return element;
}

if (SELECT_ELEMENT_INNERHTML_BUGGY || TABLE_ELEMENT_INNERHTML_BUGGY) {
var tagName = element.tagName.toUpperCase();
if (tagName in Element._insertionTranslations.tags) {
$A(element.childNodes).each(function(node) {
element.removeChild(node);
Expand Down
9 changes: 9 additions & 0 deletions test/unit/dom_test.js
Expand Up @@ -378,6 +378,15 @@ new Test.Unit.Runner({
this.assertEqual('hello world', getInnerHTML('testdiv'));
},

testElementUpdateScriptElement: function() {
var el = new Element('script', {
type: 'text/javascript'
});
this.assertNothingRaised(function(){
el.update('(function(){})');
})
},

testElementReplace: function() {
$('testdiv-replace-1').replace('hello from div!');
this.assertEqual('hello from div!', $('testdiv-replace-container-1').innerHTML);
Expand Down

5 comments on commit 71a8663

@tobie
Copy link
Collaborator

@tobie tobie commented on 71a8663 Mar 23, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a test to verify Element#update correctly evaluates the content it is passed.

@tobie
Copy link
Collaborator

@tobie tobie commented on 71a8663 Mar 23, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m a bit uneasy about this feature test. We’re interested in making sure that the content attached to a script element is evaluated. Here we’re testing that it’s doesn’t throw an error. That’s very different, imho.

@kangax
Copy link
Collaborator

@kangax kangax commented on 71a8663 Mar 23, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same concerns and actually first made a test which ensures content evaluation. The problem was Safari 2, which required `document.write` and `document.write` completely destroys our unit test runner. Any ideas on what we can do about it?

@tobie
Copy link
Collaborator

@tobie tobie commented on 71a8663 Mar 23, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should target Safari 2.0.4 (that is, fully patched version of 2.0) which solves this problem altogether.

@kangax
Copy link
Collaborator

@kangax kangax commented on 71a8663 Mar 23, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable. AIUI, this will also make it possible to implement “global eval”

Please sign in to comment.