Skip to content

Commit

Permalink
fix for ticket #119, and general script tag processing
Browse files Browse the repository at this point in the history
  • Loading branch information
client9 committed Mar 22, 2010
1 parent 17ede31 commit d07dbde
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 19 deletions.
54 changes: 39 additions & 15 deletions src/html/script.js
Expand Up @@ -7,25 +7,49 @@ HTMLScriptElement = function(ownerDocument) {
};
HTMLScriptElement.prototype = new HTMLElement;
__extend__(HTMLScriptElement.prototype, {
get text(){
// text of script is in a child node of the element
// scripts with < operator must be in a CDATA node
for (var i=0; i<this.childNodes.length; i++) {
if (this.childNodes[i].nodeType == Node.CDATA_SECTION_NODE) {
return this.childNodes[i].nodeValue;

/**
* HTML5 spec @ http://dev.w3.org/html5/spec/Overview.html#script
*
* "The IDL attribute text must return a concatenation of the
* contents of all the text nodes that are direct children of the
* script element (ignoring any other nodes such as comments or
* elements), in tree order. On setting, it must act the same way
* as the textContent IDL attribute."
*
* AND... "The term text node refers to any Text node,
* including CDATASection nodes; specifically, any Node with node
* type TEXT_NODE (3) or CDATA_SECTION_NODE (4)"
*/
get text() {
var kids = this.childNodes;
var kid;
var s = '';
var imax = kids.length;
for (var i = 0; i < imax; ++i) {
kid = kids[i];
if (kid.nodeType == Node.TEXT_NODE || kid.nodeType == Node.CDATA_SECTION_NODE) {
s += kid.nodeValue;
}
}
// otherwise there will be a text node containing the script
if (this.childNodes[0] && this.childNodes[0].nodeType == Node.TEXT_NODE) {
return this.childNodes[0].nodeValue;
}
return this.nodeValue;

return s;
},
set text(value){
this.nodeValue = value;

/**
* HTML5 spec "Can be set, to replace the element's children with
* the given value." It *does not* execute the script!

This comment has been minimized.

Copy link
@smparkes

smparkes Mar 22, 2010

Collaborator

Hmmm ... my first github comment ... I've seen jquery using it a lot ...

I think the part about not executing the script is wrong. The spec says

When a script element that is neither marked as having "already started" nor marked as being "parser-inserted" experiences one of the events listed in the following list, the user agent must synchronously run the script element:
The script element gets inserted into a document.
The script element is in a Document and its child nodes are changed.
The script element is in a Document and has a src attribute set where previously the element had no such attribute.

So in some cases we do need to run it, but we need to implement the rest of the state machine described in the spec so we don't run it in cases where we shouldn't.

*/
set text(value) {
// this deletes all children, and make a new single text node
// with value
this.textContent = value;

// it does not execute, but leaving this in for now
// only when the script is added THE FIRST time does
// this execute.
Envjs.loadInlineScript(this);
},

get htmlFor(){
return this.getAttribute('for');
},
Expand Down Expand Up @@ -65,7 +89,7 @@ __extend__(HTMLScriptElement.prototype, {
onload: HTMLEvents.prototype.onload,
onerror: HTMLEvents.prototype.onerror,
toString: function() {
return '[object HTMLScriptElement]';
return '[object HTMLScriptElement]';
}

});
Expand Down
15 changes: 12 additions & 3 deletions test/specs/html/spec.js
Expand Up @@ -305,10 +305,11 @@ test('HTMLDocument.createElement(frame)', function(){
test('HTMLDocument.createElement(script)', function(){

var element;

element = document.createElement('script');

ok(element, 'element created');
equals(element.text, '', 'text');
equals(element.childNodes.length, 0, '.childNodes.length');
equals(element.localName, 'SCRIPT', '.localName');
equals(element.namespaceURI, null, '.namespaceURI');
Expand All @@ -322,10 +323,18 @@ test('HTMLDocument.createElement(script)', function(){
equals(element.tagName, 'SCRIPT', '.tagName');
equals(xmlserializer.serializeToString(element), '<SCRIPT/>', 'xmlserializer');
debugger;

element.textContent = 'document.ASDFASDF = "QWERQWER";';
// TODO: document.ASDFASDF should still be undefined
document.head.appendChild(element);
equals(document.ASDFASDF, 'QWERQWER', 'script appended to head executes');


// create setting and getting 'text' property
element = document.createElement('script');
var s = 'var x = 1';
element.text = s;
equals(element.text, s, 'script.text');

});

// TODO: forms, input radio
Expand Down
31 changes: 30 additions & 1 deletion test/specs/window/spec.js
Expand Up @@ -332,6 +332,35 @@ test('HTMLParser.parseDocument / polluting script', function(){
document.body.removeChild( iframe );
});

/*
* Unfortunately, the exception is swallowed in html/document.js
* so we can't test it. (search for 'error loading html element')
* However this cause some error output to printed to console.
*/

test('HTMLParser.parseDocument / empty script', function(){
//one of the easiest way to test the HTMLParser is using frames and
//writing the document directly
expect(1);
var iframe = document.createElement("iframe"),
doc,
win;
document.body.appendChild(iframe);
doc = iframe.contentDocument;
win = iframe.contentWindow;

try {
doc.open();
doc.write("<html><head><script></script></head><body>hello</body></html>");
doc.close();
ok(true, 'empty script was correctly ignored');
} catch (e) {
ok(false, 'empty script causes exception:' + e);
}

});


test('frame proxy', function(){

var frame,
Expand Down Expand Up @@ -375,5 +404,5 @@ test('window.[atob|btoa]', function(){
ok(window.btoa, 'window.btoa available');

equals(window.btoa('1234'), 'MTIzNA==', 'smoke test for btoa');
equals(window.atob('MTIzNA=='), '1234', 'smoke test for btoa');
equals(window.atob('MTIzNA=='), '1234', 'smoke test for atob');
});

2 comments on commit d07dbde

@client9
Copy link
Author

Choose a reason for hiding this comment

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

I love that you are looking at my commits!

Yeah my comment-in-code is not correct either. The use case I had was

var s = document.createElement('script');
s.text = 'alert("foo");
// --does not fire--
document.addChild (or appendNode or whatever it is)(s);
// Alert!

I like tracking this in code... I will add your notes right in.

@client9
Copy link
Author

Choose a reason for hiding this comment

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

check out
http://github.com/thatcher/env-js/commit/701dd59f63f8b3a0f7fb6f34a43342add6b04577

and see if that captures the issues. thankee.

Please sign in to comment.