Fixes several failing jQuery tests (issue 5654047)
meta...@gmail.com
Description:
The lexCss() function requires a string as input but some callers
weren't
casting first. I added a cast at the start of the function.
If you wrap the onreadystatechange property of XHR in an untame(), then
when the feral native DOM layer tries to call the event callback, it
will
try to tame the |event| object passed in, which is neither possible
(it's an unsupported weirdo object) nor necessary,
so the correct thing to do is just to pass a plain function.
We do not tame xhr.responseXML, but jquery tests crash if it does not
contain at least documentElement.cloneNode. I added a kludge that
allows
the jQuery tests to continue to run and a message that Caja doesn't
support it.
attachDocumentStub() was renamed to attachDocument() a while back; this
changes the error message to match.
In one situation in jquery that I haven't been able to replicate outside
it, traversing the parentNode chain can lead to a host page DOM node
that
has not yet had the caja properties attached so that cajoled Domado can
use them. This change makes sure the properties are there.
Before this change, the HTML element violated the spec that says the
parentNode of the element should be the document, so this change
fixes that.
Please review this at http://codereview.appspot.com/5654047/
Affected files:
M src/com/google/caja/plugin/csslexer.js
M src/com/google/caja/plugin/domado.js
M tests/com/google/caja/plugin/es53-test-domado-dom-guest.html
Index: tests/com/google/caja/plugin/es53-test-domado-dom-guest.html
===================================================================
--- tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (revision
4777)
+++ tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (working
copy)
@@ -462,6 +462,7 @@
assertEquals(null, title.getAttribute('foo'));
assertEquals('HTML', html.tagName);
assertEquals(null, html.getAttribute('foo'));
+ assertEquals(html.parentNode, document);
assertEquals(html, all[0]);
assertEquals(head, all[1]);
assertEquals(title, all[2]);
Index: src/com/google/caja/plugin/csslexer.js
===================================================================
--- src/com/google/caja/plugin/csslexer.js (revision 4777)
+++ src/com/google/caja/plugin/csslexer.js (working copy)
@@ -223,6 +223,7 @@
* delimiters and to not otherwise contain double quotes.
*/
lexCss = function (cssText) {
+ cssText = '' + cssText;
var tokens = cssText.replace(/\r\n?/g, '\n') // Normalize CRLF & CR
to LF.
.match(CSS_TOKEN) || [];
var j = 0;
Index: src/com/google/caja/plugin/domado.js
===================================================================
--- src/com/google/caja/plugin/domado.js (revision 4777)
+++ src/com/google/caja/plugin/domado.js (working copy)
@@ -618,10 +618,10 @@
// 'target'? May need to implement full "tame event" wrapper
similar
// to DOM events.
var self = this;
- p(this).feral.onreadystatechange = taming.untame(function
(event) {
+ p(this).feral.onreadystatechange = function (event) {
var evt = { target: self };
return handler.call(void 0, evt);
- });
+ };
// Store for later direct invocation if need be
p(this).handler = handler;
})
@@ -647,7 +647,14 @@
// TODO(ihab.awad): Implement a taming layer for XML. Requires
// generalizing the HTML node hierarchy as well so we have a
unified
// implementation.
- return {};
+
+ // This kludge is just enough to keep the jQuery tests from
freezing.
+ var node = {nodeName: '#document'};
+ node.cloneNode = function () { return node; };
+ node.toString = function () {
+ return 'Caja does not support XML.';
+ };
+ return {documentElement: node};
})
},
status: {
@@ -719,7 +726,7 @@
// TODO(ihab.awad): Expect tamed XML document; unwrap and send
p(this).feral.send('');
}
-
+
// Firefox does not call the 'onreadystatechange' handler in
// the case of a synchronous XHR. We simulate this behavior by
// calling the handler explicitly.
@@ -1299,7 +1306,7 @@
idSuffix, uriCallback, pseudoBodyNode, optPseudoWindowLocation) {
if (arguments.length < 3) {
throw new Error(
- 'attachDocumentStub arity mismatch: ' + arguments.length);
+ 'attachDocument arity mismatch: ' + arguments.length);
}
if (!optPseudoWindowLocation) {
optPseudoWindowLocation = {};
@@ -1960,7 +1967,9 @@
// Catch errors because node might be from a different domain.
try {
var docElem = node.ownerDocument.documentElement;
- for (var ancestor = node; ancestor; ancestor =
ancestor.parentNode) {
+ for (var ancestor = node;
+ ancestor;
+ ancestor = makeDOMAccessible(ancestor.parentNode)) {
if (idClassPattern.test(ancestor.className)) {
return tameNodeCtor(node, editable);
} else if (ancestor === docElem) {
@@ -4653,7 +4662,7 @@
'HTML',
this,
function () { return [tameHeadElement, tameBodyElement]; },
- function () { return tamingProxies.get(this); },
+ function () { return tameDocument; },
function () {
return ('<head>' + tameHeadElement.innerHTML
+ '<\/head><body>'
@@ -5265,14 +5274,16 @@
* See
http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#window
for the full API.
*/
function TameWindow() {
- // These descriptors were chosen to resemble actual ES5-supporting
browser
- // behavior.
+ // Exposed as an accessor since we need to close over the
reference to
+ // tameDocument rather than the value, which is replaced later.
Object.defineProperty(this, "document", {
- value: tameDocument,
- configurable: false,
+ get: function () { return tameDocument; },
+ set: void 0,
enumerable: true,
- writable: false
+ configurable: false
});
+ // These descriptors were chosen to resemble actual ES5-supporting
browser
+ // behavior.
Object.defineProperty(this, "location", {
value: tameLocation,
configurable: false,
ihab...@gmail.com
meta...@gmail.com
meta...@gmail.com
tests. I was able to fix them by defining the document property lower
in the file. Can I get another LGTM?