It doesn't address the issue. The issue was bogus. This addresses a different correctness issue : that we were exposing the virtual body as a tamed DIV element instead of a tamed BODY element.
2009/9/18 Jasvir Nagra <[email protected]>: > I don't follow how the changed tameRelatedNode addresses the issue (even > though I see from the test that it does) - it looks like a > semantics-preserving change to me. Can you add a small explanation for my > benefit. > > On Fri, Sep 18, 2009 at 1:02 PM, Mike Samuel <[email protected]> wrote: >> >> The description of this CL is misleading. I added a test, and then >> realized that tameRelated was buggy. Changed the description in >> rietveld. >> >> 2009/9/18 <[email protected]>: >> > Reviewers: jasvir, >> > >> > Description: >> > http://code.google.com/p/google-caja/issues/detail?id=1126 >> > >> > offsetParent is exposed. >> > >> > Please review this at http://codereview.appspot.com/120049 >> > >> > Affected files: >> > M src/com/google/caja/plugin/domita.js >> > M tests/com/google/caja/plugin/domita_test_untrusted.html >> > >> > >> > Index: tests/com/google/caja/plugin/domita_test_untrusted.html >> > =================================================================== >> > --- tests/com/google/caja/plugin/domita_test_untrusted.html >> > (revision >> > 3731) >> > +++ tests/com/google/caja/plugin/domita_test_untrusted.html (working >> > copy) >> > @@ -237,6 +237,10 @@ >> > >> > <p class="testcontainer" id="test-inner-HTML-style-sanitizer"></p> >> > >> > +<p class="testcontainer" id="test-offset-parent" >> > style="position:relative"> >> > +<span id="test-offset-parent-inner">inner</span> >> > +</p> >> > + >> > <div class="testcontainer" id="test-whitespace-nodes-1"> >> > <div style="width: 150px; border: 1px solid blue; margin: 0; padding: >> > 2px" >> > id="whitespace-outer"> >> > @@ -1775,6 +1779,19 @@ >> > pass('test-inner-HTML-style-sanitizer'); >> > }); >> > >> > +jsunitRegister('testOffsetParent', >> > + function testOffsetParent() { >> > + var outer = document.getElementById('test-offset-parent'); >> > + var inner = document.getElementById('test-offset-parent-inner'); >> > + assertEquals("wrong inner.offsetParent: " + inner.offsetParent, >> > + outer, inner.offsetParent); >> > + assertEquals("wrong outer.offsetParent: " + outer.offsetParent, >> > + document.body, outer.offsetParent); >> > + assertEquals("wront body.offsetParent: " + >> > document.body.offsetParent, >> > + undefined, document.body.offsetParent); >> > + pass('test-offset-parent'); >> > +}); >> > + >> > function assertWithin(msg, golden, actual, tolerance) { >> > if (!(Math.abs(golden - actual) <= tolerance)) { >> > assertEquals(msg, golden, actual); >> > @@ -2989,15 +3006,15 @@ >> > Q.when(m, f1, f2); >> > } else { >> > pass(prefix + '-unbundled-module-loader'); >> > - } >> > + } >> > } >> > >> > -jsunitRegister('testXhrUnbundledModuleLoader', >> > +jsunitRegister('testXhrUnbundledModuleLoader', >> > function testXhrUnbundledModuleLoader() { >> > testUnbundledModuleLoader(xhrModuleLoad, "xhr"); >> > }); >> > >> > -jsunitRegister('testScriptUnbundledModuleLoader', >> > +jsunitRegister('testScriptUnbundledModuleLoader', >> > function testScriptUnbundledModuleLoader() { >> > testUnbundledModuleLoader(scriptModuleLoad, "script"); >> > }); >> > Index: src/com/google/caja/plugin/domita.js >> > =================================================================== >> > --- src/com/google/caja/plugin/domita.js (revision 3731) >> > +++ src/com/google/caja/plugin/domita.js (working copy) >> > @@ -1054,6 +1054,14 @@ >> > >> > function tameRelatedNode(node, editable, tameNodeCtor) { >> > if (node === null || node === void 0) { return null; } >> > + if (node === tameDocument.body___) { >> > + if (tameDocument.editable___ && !editable) { >> > + // FIXME: return a non-editable version of body. >> > + throw new Error(NOT_EDITABLE); >> > + } >> > + return tameDocument.getBody(); >> > + } >> > + >> > // Catch errors because node might be from a different domain. >> > try { >> > var docElem = node.ownerDocument.documentElement; >> > @@ -1484,15 +1492,8 @@ >> > return defaultTameNode(this.node___.previousSibling, >> > this.editable___); >> > }; >> > TameBackedNode.prototype.getParentNode = function () { >> > - var parent = this.node___.parentNode; >> > - if (parent === tameDocument.body___) { >> > - if (tameDocument.editable___ && !this.editable___) { >> > - // FIXME: return a non-editable version of body. >> > - throw new Error(NOT_EDITABLE); >> > - } >> > - return tameDocument.getBody(); >> > - } >> > - return tameRelatedNode(parent, this.editable___, >> > defaultTameNode); >> > + return tameRelatedNode( >> > + this.node___.parentNode, this.editable___, defaultTameNode); >> > }; >> > TameBackedNode.prototype.getElementsByTagName = function (tagName) { >> > return tameGetElementsByTagName( >> > >> > >> > > >
