Reviewers: felix8a,
Description:
* Implement HTML5-conformant behavior for document.title if there is no
<title> element.
* Remove attribute/property "blacklist" code from TameIFrameElement
which is no longer necessary since the whitelist is per-attribute.
* Move misplaced @param comment for taming membrane parameter.
Please review this at http://codereview.appspot.com/6843122/
Affected files:
M src/com/google/caja/plugin/domado.js
M tests/com/google/caja/plugin/es53-test-domado-dom-guest.html
M tests/com/google/caja/plugin/es53-test-domado-global.js
Index: tests/com/google/caja/plugin/es53-test-domado-dom-guest.html
===================================================================
--- tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (revision
5166)
+++ tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (working
copy)
@@ -2497,8 +2497,8 @@
assertEquals('1', ifr.getAttribute('frameBorder'));
assertEquals(null, ifr.getAttribute('name'));
assertEquals(null, ifr.getAttribute('src'));
- assertEquals(void 0, ifr.name);
- assertEquals(void 0, ifr.src);
+ assertEquals(null, ifr.name);
+ assertEquals(null, ifr.src);
pass('testIframeShim');
});
</script>
Index: tests/com/google/caja/plugin/es53-test-domado-global.js
===================================================================
--- tests/com/google/caja/plugin/es53-test-domado-global.js (revision 5166)
+++ tests/com/google/caja/plugin/es53-test-domado-global.js (working copy)
@@ -105,6 +105,29 @@
}());
/**
+ * Tests of global structure-referencing property behavior.
+ */
+ (function () {
+ // Behavior of document.title without any title element present
+ registerGuestTest('testNoElementTitleProp',
+ '<body>' + // TODO(kpreid): That this is required is a
HtmlEmitter bug
+ '<script>window.globalGuestTest =' +
+ ' function globalGuestTest() {' +
+ ' assertEquals("head before get", "", ' +
+ ' document.getElementsByTagName("head")[0].innerHTML);' +
+ ' assertEquals("title before set", "", document.title);' +
+ ' assertEquals("head after get", "", ' +
+ ' document.getElementsByTagName("head")[0].innerHTML);' +
+ ' document.title = "t";' +
+ ' assertEquals("title after set", "t", document.title);' +
+ ' assertEquals("head after set", "<title>t</title>", ' +
+ ' document.getElementsByTagName("head")[0].innerHTML);' +
+ '};</script>');
+
+ // TODO(kpreid): Test document.body
+ })();
+
+ /**
* Tests of onload handlers, which are a special case because on*
attributes
* on <body> are actually aliases for event handlers of the _window_
object,
* so since our virtual window object is not backed by anything we
have to
Index: src/com/google/caja/plugin/domado.js
===================================================================
--- src/com/google/caja/plugin/domado.js (revision 5166)
+++ src/com/google/caja/plugin/domado.js (working copy)
@@ -922,7 +922,6 @@
* objects, and a shared map allowing separate virtual documents to
dispatch
* events across them. (TODO(kpreid): Confirm this explanation is good.)
*
- * @param {Object} taming. An interface to a taming membrane.
* @param {Object} opt_rulebreaker. If necessary, authorities to break
the
* ES5/3 taming membrane and work with the taming-frame system. If
* running under SES, pass null instead.
@@ -1519,6 +1518,7 @@
* virtual Document node provided to Cajoled code.
* @param {Object} optTargetAttributePresets a record containing the
presets
* (default and whitelist) for the HTML "target" attribute.
+ * @param {Object} taming. An interface to a taming membrane.
* @return {Object} A collection of privileged access tools, plus the
tamed
* {@code document} and {@code window} objects under those names.
This
* object is known as a "domicile".
@@ -3576,6 +3576,11 @@
bridal.getAttribute(feral, 'target')));
}
}
+ } else {
+ if (typeof console !== 'undefined') {
+ console.warn('Rejecting <' + tagName + '>.setAttribute(',
+ attribName, ',', value, ')');
+ }
}
}
return value;
@@ -4618,15 +4623,6 @@
domClass: 'HTMLHtmlElement'
});
- var P_blacklist = {
- enumerable: true,
- extendedAccessors: true,
- get: nodeMethod(function () { return undefined; }),
- set: nodeMethod(function (value, prop) {
- if (typeof console !== 'undefined')
- console.error('Cannot set the [', prop, '] property of an
iframe.');
- })
- };
var TameIFrameElement = defineElement({
domClass: 'HTMLIFrameElement',
construct: function () {
@@ -4664,8 +4660,8 @@
},
height: NP.filterProp(identity, Number),
width: NP.filterProp(identity, Number),
- src: P_blacklist,
- name: P_blacklist,
+ src: NP.filterAttr(identity, identity), // rewrite handled for
attr
+ name: NP.filterAttr(identity, identity), // rejection handled
for attr
contentDocument: {
enumerable: true,
get: cajaVM.def(function () {
@@ -4693,18 +4689,10 @@
setOwn(TameIFrameElement.prototype, 'setAttribute',
nodeMethod(function (attr, value) {
var attrLc = String(attr).toLowerCase();
- // The 'name' and 'src' attributes are whitelisted for all tags in
- // html4-attributes-whitelist.json, since they're needed on tags
- // like <img>. Because there's currently no way to filter
attributes
- // based on the tag, we have to blacklist these two here.
- // TODO(kpreid): Don't we have per-attribute filtering now?
- if (attrLc !== 'name' && attrLc !== 'src') {
- return TameElement.prototype.setAttribute.call(this, attr,
value);
+ if (attrLc === 'src') {
+ // ...
}
- if (typeof console !== 'undefined')
- console.error('Cannot set the [' + attrLc +
- '] attribute of an iframe.');
- return value;
+ return TameElement.prototype.setAttribute.call(this, attr, value);
}));
function contentDomicile(tameIFrame) {
// TODO(kpreid): Once we support loading content via src=, we will
need
@@ -5318,10 +5306,19 @@
enumerable: true,
get: nodeMethod(function() {
var titleEl = this.getElementsByTagName('title')[0];
- return trimHTML5Spaces(titleEl.textContent);
+ return titleEl ? trimHTML5Spaces(titleEl.textContent) : "";
}),
set: nodeMethod(function(value) {
var titleEl = this.getElementsByTagName('title')[0];
+ if (!titleEl) {
+ var head = this.getElementsByTagName('head')[0];
+ if (head) {
+ titleEl = this.createElement('title');
+ head.appendChild(titleEl);
+ } else {
+ return;
+ }
+ }
titleEl.textContent = value;
})
},
