From 8200a629a7e15ebb56e2c1c89fbba61c5ab4db8c Mon Sep 17 00:00:00 2001
From: jakearchibald <jake.archibald@bbc.co.uk>
Date: Fri, 25 Sep 2009 13:22:13 +0100
Subject: [PATCH] Changes following review

net: changed redirect test/glow/embed/embed.js Embed: Making test clearer
Unit tests: adding line breaks after skip
dom: added comment
net: Adding comment for net memory leak
---
 src/dom/dom.js           |  1 +
 src/net/net.js           |  2 ++
 test/glow/embed/embed.js | 17 ++++++++++++-----
 test/glow/glow.js        |  5 ++++-
 test/glow/net/net.js     | 27 +++++++++++++++++----------
 5 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/src/dom/dom.js b/src/dom/dom.js
index ae0b1dc..d9fcf11 100755
--- a/src/dom/dom.js
+++ b/src/dom/dom.js
@@ -2510,6 +2510,7 @@
 					length = this.length;
 
 				if (newHtml !== undefined) {
+					// not setting innerHTML, doesn't work in IE for elements like <table>
 					return this.empty().append(newHtml);
 				}
 				return this[0] ? this[0].innerHTML : "";
diff --git a/src/net/net.js b/src/net/net.js
index 7691c5d..4eb8882 100644
--- a/src/net/net.js
+++ b/src/net/net.js
@@ -147,6 +147,7 @@
 							} else {
 								events.fire(request, "error", response);
 							}
+							// prevent parent scopes leaking (cross-page) in IE
 							req.onreadystatechange = new Function();
 						}
 					};
@@ -375,6 +376,7 @@
 			//different if request came from loadScript
 			if (nativeReq) {
 				//clear listeners
+				// prevent parent scopes leaking (cross-page) in IE
 				nativeReq.onreadystatechange = new Function();
 				nativeReq.abort();
 			} else if (callbackIndex) {
diff --git a/test/glow/embed/embed.js b/test/glow/embed/embed.js
index b7dcbb0..90db92b 100644
--- a/test/glow/embed/embed.js
+++ b/test/glow/embed/embed.js
@@ -36,7 +36,8 @@ t.test("Test Embedding", function() {
 	
 	if (!movie.movie) {
 		flashHolder.destroy();
-		t.skip("Flash 9 or greater required"); return;
+		t.skip("Flash 9 or greater required");
+		return;
 	}
 	
 	t.ok(movie.movie.nodeName.toLowerCase() == embed_tag, "Movie property references Flash")
@@ -147,7 +148,8 @@ t.test("check that supplied attributes are set correctly", function() {
 	
 	if (!movie.movie) {
 		flashHolder.destroy();
-		t.skip("Flash 6 or greater required"); return;
+		t.skip("Flash 6 or greater required");
+		return;
 	}
 	
 	t.ok($("#testFlash")[0].tagName.toLowerCase() == embed_tag,"set id");
@@ -181,7 +183,8 @@ t.test("Shortcut attribute setting", function() {
   
 	if (!myFlash.movie) {
 		flashHolder.destroy();
-		t.skip("Flash 6 or greater required"); return;
+		t.skip("Flash 6 or greater required");
+		return;
 	}
 	
 	t.equals(myFlash.movie.id, "newIdValue", "ID");
@@ -202,13 +205,17 @@ t.test("check correct defaults for missing parameters", function() {
 	
 	if (!movie.movie) {
 		flashHolder.destroy();
-		t.skip("Flash 6 or greater required"); return;
+		t.skip("Flash 6 or greater required");
+		return;
 	}
 	
 	try {
 		//not using getAttribute for the first two as some browsers give empty strings while others give null
 		t.ok(embed.id != "","Movie has auto-generated ID");
-		t.ok(embed.name == "" || embed.name == undefined,"no name by default");
+		
+		var hasName = embed.name != "" || embed.name != undefined;
+		
+		t.ok( !hasName, "no name by default" );
 		if (glow.env.ie) {
 		  t.ok($(embed).children().filter(function() { return this.name == "allowscriptaccess" }).attr("value") == "always", "allowscriptaccess:always");
 		} else {
diff --git a/test/glow/glow.js b/test/glow/glow.js
index 2fa6a4b..8ff988f 100644
--- a/test/glow/glow.js
+++ b/test/glow/glow.js
@@ -8,7 +8,10 @@ t.test("Basic requirements", function() {
 
 t.test("Simple DOM Ready testing", function() {
 	t.expect(4);
-	if (glow.isReady) { t.skip("DOM already ready"); return; }
+	if (glow.isReady) {
+		t.skip("DOM already ready");
+		return;
+	}
 	t.stop();
 
 	var firstFired = false;
diff --git a/test/glow/net/net.js b/test/glow/net/net.js
index 9af53ab..d64ac80 100644
--- a/test/glow/net/net.js
+++ b/test/glow/net/net.js
@@ -68,7 +68,8 @@ t.test("glow.net.get async header setting", function() {
 		onLoad: function(response) {
 			if ( response.text().slice(0, 2) == '<?' ) {
 				t.start();
-				t.skip("This test requires a web server running PHP5"); return;
+				t.skip("This test requires a web server running PHP5");
+				return;
 			}
 			t.ok(true, "correct callback used");
 			t.ok(/^REQUEST_METHOD: GET/m.test(response.text()), "Using get method");
@@ -182,7 +183,8 @@ t.test("glow.net.post async string", function() {
 			onLoad: function(response) {
 				if ( response.text().slice(0, 2) == '<?' ) {
 					t.start();
-					t.skip("This test requires a web server running PHP5"); return;
+					t.skip("This test requires a web server running PHP5");
+					return;
 				}
 				t.ok(true, "correct callback used");
 				t.equals( (/^REQUEST_METHOD: (\w+)/m.exec(response.text()) || [,,])[1], "POST", "Using post method" );
@@ -206,7 +208,8 @@ t.test("glow.net.post aync json", function() {
 			onLoad: function(response) {
 				if ( response.text().slice(0, 2) == '<?' ) {
 					t.start();
-					t.skip("This test requires a web server running PHP5"); return;
+					t.skip("This test requires a web server running PHP5");
+					return;
 				}
 				t.ok(true, "correct callback used");
 				t.equals( (/^REQUEST_METHOD: (\w+)/m.exec(response.text()) || [,,])[1], "POST", "Using post method" );
@@ -289,17 +292,19 @@ t.test("glow.net.get defering and multiple error events", function() {
 
 
 //test redirects
-for (var i = 1; i < 5; i++) {
-	if (i == 4) i = 7;
+var redirectCodes = [301, 302, 303, 307];
+
+for (var i = 0, len = redirectCodes.length; i < len; i++) {
 	(function(i) {
-		t.test("glow.net.get redirect 30" + i, function() {
+		t.test("glow.net.get redirect " + redirectCodes[i], function() {
 			t.expect(2);
 			t.stop();
-			glow.net.get("testdata/xhr/redirect.php?code=" + i, {
+			glow.net.get("testdata/xhr/redirect.php?code=" + redirectCodes[i], {
 				onLoad: function(response) {
 					if ( response.text().slice(0,2) == '<?' ) {
 						t.start();
-						t.skip("This test requires a web server running PHP5"); return;
+						t.skip("This test requires a web server running PHP5");
+						return;
 					}
 					t.ok(true, "onLoad called");
 					t.equals(response.text(), "XHR Test Document", "File returned");
@@ -308,7 +313,8 @@ for (var i = 1; i < 5; i++) {
 				onError: function(response) {
 					if ( response.text().slice(0,2) == '<?' ) {
 						t.start();
-						t.skip("This test requires a web server running PHP5"); return;
+						t.skip("This test requires a web server running PHP5");
+						return;
 					}
 					t.ok(false, "onLoad called");
 					t.equals(response.text(), "XHR Test Document", "File returned");
@@ -407,7 +413,8 @@ t.test("glow.net.loadScript aborting", function() {
 		timeout: 2
 	});
 	if (request.completed) {
-		t.skip("Request complete, too late to abort"); return;
+		t.skip("Request complete, too late to abort");
+		return;
 	}
 	request.abort();