Revision: 3857
Author: felix8a
Date: Thu Nov 19 23:47:53 2009
Log: fix various domita test issues
http://codereview.appspot.com/157093
This fixes:
bug 1131 - bridal constructClone doesn't set the "class"
attribute properly on IE[67].
bug 1140 - testUseMap fails on various browsers, because it's
doing un-normalized html comparison. after this change, it
still fails on IE[67] because you can't set an element's @name
after it's created.
bug 1142 - two cloneNode tests fail on IE8-std.
bug 1143 - _testSideEffectTestFramework fails on IE.
bug 1162 - Array.slice behaves differently on various browsers
when the end arg === undefined.
bug 1168 - "Ready for testing" box never turns green on IE[67].
[email protected]
http://code.google.com/p/google-caja/source/detail?r=3857
Modified:
/trunk/src/com/google/caja/cajita.js
/trunk/src/com/google/caja/plugin/bridal.js
/trunk/tests/com/google/caja/plugin/domita_test.html
/trunk/tests/com/google/caja/plugin/domita_test_untrusted.html
=======================================
--- /trunk/src/com/google/caja/cajita.js Tue Nov 17 11:07:08 2009
+++ /trunk/src/com/google/caja/cajita.js Thu Nov 19 23:47:53 2009
@@ -87,16 +87,24 @@
}
/**
- * Provide Array.slice like Firefox. You can slice anything, and the
- * result is always an array. This behaves differently from Firefox in the
- * exotic case of an array-like x with typeof x !== 'object'.
+ * Provide Array.slice similar to Firefox. You can slice anything, and the
+ * result is always an array. Hazards:
+ *
+ * - In IE, Array.prototype.slice.call(void 0) throws an error, and we
+ * need it to return [].
+ *
+ * - In IE[678] and Firefox 3, x.slice(0, undefined) returns [] rather
+ * than x. We don't care about that incompatibility, but we do need
+ * Array.slice(x) to return x.
+ *
+ * - In Firefox 3.x, Array.slice works on any array-like x. We only
+ * handle typeof x === 'object'.
*/
if (Array.slice === void 0) {
- Array.slice = function(self, start, end) {
+ Array.slice = function(self, opt_start, opt_end) {
if (self && typeof self === 'object') {
- if (arguments.length < 2) { start = 0; }
- if (arguments.length < 3) { end = self.length; }
- return Array.prototype.slice.call(self, start, end);
+ if (opt_end === void 0) { opt_end = self.length; }
+ return Array.prototype.slice.call(self, opt_start, opt_end);
} else {
return [];
}
=======================================
--- /trunk/src/com/google/caja/plugin/bridal.js Fri Nov 13 13:37:15 2009
+++ /trunk/src/com/google/caja/plugin/bridal.js Thu Nov 19 23:47:53 2009
@@ -36,7 +36,6 @@
var isWebkit = !isOpera && navigator.userAgent.indexOf('WebKit') !== -1;
var featureAttachEvent = !!(document.createElement('div').attachEvent);
- var featureSetAttributeExtraParam = isIE;
/**
* Does the extended form of extendedCreateElement work?
* From
http://msdn.microsoft.com/en-us/library/ms536389.aspx :<blockquote>
@@ -134,7 +133,7 @@
var attrs = node.attributes;
for (var i = 0, attr; (attr = attrs[i]); ++i) {
if (attr.specified && !endsWith__.test(attr.name)) {
- clone.setAttribute(attr.nodeName, attr.nodeValue);
+ bridal.setAttribute(clone, attr.nodeName, attr.nodeValue);
}
}
} else {
=======================================
--- /trunk/tests/com/google/caja/plugin/domita_test.html Fri Nov 13
13:37:15 2009
+++ /trunk/tests/com/google/caja/plugin/domita_test.html Thu Nov 19
23:47:53 2009
@@ -66,7 +66,7 @@
function _testSideEffectTestFramework() {
assertFalse(checkGlobalSideEffect());
var s = document.createElement('SCRIPT');
- s.innerHTML = 'globalSideEffect()';
+ s.text = 'globalSideEffect()';
document.body.appendChild(s);
assertTrue(checkGlobalSideEffect());
assertFalse(checkGlobalSideEffect());
@@ -164,7 +164,7 @@
function readyForAutomatedTesting() {
var e = document.getElementById('automatedTestingReadyIndicator');
- e.setAttribute('class', 'readytotest');
+ bridal.setAttribute(e, 'class', 'readytotest');
e.innerHTML = 'Ready for testing';
}
@@ -458,6 +458,8 @@
});
window.toString = function () { return '[WINDOW]'; };
+ document.title += isValija ? ' valija' : ' cajita';
+
inlineHtml(
isValija
? 'domita_test_untrusted_valija.out.html'
@@ -468,10 +470,8 @@
this.evaluateAsyncRequirements(function (pass) {
if (!pass) {
document.title = document.title.replace(
- / - all tests passed$/, ' - async tests failed');
- }
- assertTrue(document.title,
- /(?: - all tests passed)?$/.test(document.title));
+ /all tests passed/, 'async tests failed');
+ }
});
readyForAutomatedTesting();
=======================================
--- /trunk/tests/com/google/caja/plugin/domita_test_untrusted.html Fri Nov
13 13:37:15 2009
+++ /trunk/tests/com/google/caja/plugin/domita_test_untrusted.html Thu Nov
19 23:47:53 2009
@@ -329,7 +329,7 @@
<div class="testcontainer" id="test-strange-ids"></div>
<div class="testcontainer" id="test-clone-node-structure">
- <p>I have <b title="I have attributes">content</b></p>
+ <p class="cloneable">some <label for="x">content</label></p>
</div>
<p class="testcontainer" id="test-compat-mode">Compat Mode:</p>
@@ -635,8 +635,12 @@
}
/**
- * Canonicalize well formed innerHTML output, by making sure that
attributes
- * are ordered by name, and have quoted values.
+ * Canonicalize innerHTML output:
+ * - collapse all whitespace to a single space
+ * - remove whitespace between adjacent tags
+ * - lowercase tagnames and attribute names
+ * - sort attributes by name
+ * - quote attribute values
*
* Without this step, it's impossible to compare innerHTML cross-browser.
*/
@@ -656,7 +660,7 @@
function (_, tagStart, tagBody) {
var attrs = [];
for (var m; tagBody && (m = tagBody.match(htmlAttribute));) {
- var name = m[1];
+ var name = m[1].toLowerCase();
var value = m[2];
var hasValue = value != null;
if (hasValue && (new RegExp('^["\']')).test(value)) {
@@ -669,15 +673,18 @@
tagBody = tagBody.substring(m[0].length);
}
attrs.sort();
- return tagStart + ' ' + attrs.join(' ') + '>';
+ attrs.unshift(tagStart);
+ return attrs.join(' ') + '>';
});
s = s.replace(
htmlTag,
function (_, open, name, body) {
return open + name.toLowerCase() + (body || '') + '>';
});
- // Remove ignorable whitespace.
- s = s.replace(ignorableWhitespace, '');
+ // Collapse whitespace.
+ s = s.replace(new RegExp('\\s+', 'g'), ' ');
+ s = s.replace(new RegExp('^ | $', 'g'), '');
+ s = s.replace(new RegExp('[>]\\s+[<]', 'g'), '><');
// Normalize escaping of text nodes since Safari doesn't escape loose >.
s = s.replace(
tagEntityOrText,
@@ -1009,7 +1016,8 @@
jsunitRegister('testDocumentWrite',
function testDocumentWrite() {
// TODO(mikesamuel): This is wrong because we don't allow the insertion
point
- // to move above it's starting position
+ // to move above its starting position
+ var el = document.getElementById('test-document-write');
assertEquals(
''
+ '<div>'
@@ -1021,8 +1029,7 @@
+ '<p>Written afterwards</p>'
+ '<p>More written afterwards</p>'
+ '</div>',
- document.getElementById('test-document-write').innerHTML
- .replace(new RegExp('[\\r\\n]\\s*', 'g'), ''));
+ canonInnerHtml(el.innerHTML));
pass('test-document-write');
});
@@ -1812,18 +1819,29 @@
}
}
+ // These cases are critical, because they affect the correctness of
+ // valija's fn.apply() implementation. IE is the problem case, because
+ // it throws an error when you Array.prototype.slice.call(null).
assertArray([], Array.slice(null), 'null');
assertArray([], Array.slice(void 0), 'void 0');
- // note difference between missing arg and undefined arg.
+ // These are expected behavior that work fine on all browsers.
assertArray([9], Array.slice([9]), '([9])');
+ assertArray([9], Array.slice([9], null), '([9], null)');
assertArray([9], Array.slice([9], void 0), '([9], void 0)');
- assertArray([], Array.slice([9], void 0, void 0), '([9], void 0, void
0)');
+ assertArray([], Array.slice([9], null, 0), '([9], null, 0)');
assertArray([], Array.slice([9], void 0, 0), '([9], void 0, 0)');
+ assertArray([], Array.slice([9], null, null), '([9], null, null)');
+ assertArray([], Array.slice([9], void 0, null), '([9], void 0, null)');
assertArray([2,3,4], Array.slice([0,1,2,3,4,5,6], 2, 5));
assertArray([2,3], Array.slice([0,1,2,3], 2, 5));
+ // es5 says end===undefined is the same as end===length, and most
+ // browsers agree. Firefox 3.x disagrees and treats it as end===0.
+ // Nothing depends on this behavior, so we ignore the incompatibility.
+ //assertArray([9], Array.slice([9], null, void 0), '([9], null, void
0)');
+
pass('test-array-slice');
});
@@ -2378,7 +2396,9 @@
{
nodeType: 1,
tagName: 'P',
- attributes: [],
+ attributes: [
+ { nodeName: 'class', nodeValue: 'cloneable' }
+ ],
children: [
// shallow clones contain attrs but not children.
]
@@ -2388,20 +2408,19 @@
{
nodeType: 1,
tagName: 'P',
- attributes: [],
+ attributes: [
+ { nodeName: 'class', nodeValue: 'cloneable' }
+ ],
children: [
{
nodeType: 3,
- nodeValue: 'I have '
+ nodeValue: 'some '
},
{
nodeType: 1,
- nodeName: 'B',
+ nodeName: 'LABEL',
attributes: [
- {
- nodeName: 'title',
- nodeValue: 'I have attributes'
- }
+ { nodeName: 'for', nodeValue: 'x' }
],
children: [
{
@@ -3153,6 +3172,7 @@
jsunitRegister('testUsemap',
function testUsemap() {
+ var el = document.getElementById('test-usemap');
assertEquals(
(''
+ '<map name="foo-xyz___">'
@@ -3160,10 +3180,13 @@
+ '?mime-type=*%2F*&uri=areatarget.html"'
+ ' target="_blank">'
+ '</map>'
- + '<img usemap="#foo-xyz___"'
- + '
src="http://example.com/?mime-type=image%2F*&uri=mappic.gif";>'),
- directAccess.getInnerHTML(document.getElementById('test-usemap')));
- pass('test-usemap');
+ + '<img'
+ + '
src="http://example.com/?mime-type=image%2F*&uri=mappic.gif";'
+ + ' usemap="#foo-xyz___"'
+ + '>'),
+ canonInnerHtml(directAccess.getInnerHTML(el)));
+
+ pass('test-usemap');
});
jsunitRegister('testDocumentBodyAppendChild',
![http://example.com/?mime-type=image%2F*&uri=mappic.gif"](http://example.com/?mime-type=image%2F*&uri=mappic.gif"){kind=link}