Reviewers: MikeSamuel,
Description:
this fixes http://code.google.com/p/google-caja/issues/detail?id=1171
YUI was failing on IE[67] when trying to create
a YUI button using an existing <button>,
because it was trying to setAttribute("type", "button")
and that normally just fails silently on IE[67],
but throws an exception when using bridal.
Please review this at http://codereview.appspot.com/159053
Affected files:
M src/com/google/caja/plugin/bridal.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 3858)
+++ tests/com/google/caja/plugin/domita_test_untrusted.html (working copy)
@@ -435,6 +435,10 @@
<p class="testcontainer" id="test-for-attribute">for attribute</p>
+<p class="testcontainer" id="test-button-mutation">
+<button id="test-button-mutation-1">test-button-mutation</button>
+</p>
+
<div class="testcontainer" id="test-row-cell">
Test row cell
<table id="test-row-cell-1">
@@ -3068,6 +3072,17 @@
pass('test-for-attribute');
});
+jsunitRegister('testButtonMutation', function testButtonMutation() {
+ var el = document.getElementById('test-button-mutation-1');
+ // this never works on IE[67], but it shouldn't throw an error.
+ try {
+ el.setAttribute('type', 'reset');
+ pass('testButtonMutation');
+ } catch (e) {
+ fail('testButtonMutation got error: ' + e.message);
+ }
+});
+
jsunitRegister('testRowCell',
function testRowCell() {
var table = document.getElementById('test-row-cell-1');
Index: src/com/google/caja/plugin/bridal.js
===================================================================
--- src/com/google/caja/plugin/bridal.js (revision 3858)
+++ src/com/google/caja/plugin/bridal.js (working copy)
@@ -393,19 +393,48 @@
* @param {string} value the value of an attribute.
*/
function setAttribute(element, name, value) {
- // In IE[67], element.style.cssText seems to be the only way to set the
- // style. This unfortunately fails when element.style is an input
- // element instead of the style object.
- if (name === 'style') {
- if (typeof element.style.cssText === 'string') {
- element.style.cssText = value;
- return value;
- }
+ /*
+ Hazards:
+
+ - In IE[67], el.setAttribute doesn't work for attributes like
+ 'class' or 'for'. IE[67] expects you to set 'className' or
+ 'htmlFor'. Using setAttributeNode solves this problem.
+
+ - In IE[67], <input> elements can shadow attributes. If el is a
+ form that contains an <input> named x, then el.setAttribute(x, y)
+ will set x's value rather than setting el's attribute. Using
+ setAttributeNode solves this problem.
+
+ - In IE[67], the style attribute can only be modified by setting
+ el.style.cssText. Neither setAttribute nor setAttributeNode will
+ work. el.style.cssText isn't bullet-proof, since it can be
+ shadowed by <input> elements.
+
+ - In IE[67], you can never change the type of an <button> element.
+ setAttribute('type') silently fails, but setAttributeNode
+ throws an exception. We want the silent failure.
+
+ - In IE[67], you can never change the type of an <input> element.
+ setAttribute('type') throws an exception. We want the exception.
+
+ - In IE[67], setAttribute is case-sensitive, unless you pass 0 as a
+ 3rd argument. setAttributeNode is case-insensitive.
+
+ - Trying to set an invalid name like ":" is supposed to throw an
+ error. In IE[678] and Opera 10, it fails without an error.
+ */
+ if (name === 'style' && typeof element.style.cssText === 'string') {
+ element.style.cssText = value;
+ return value;
}
- var node = element.ownerDocument.createAttribute(name);
- node.value = value;
- element.setAttributeNode(node);
- return value;
+ var attr = element.ownerDocument.createAttribute(name);
+ attr.value = value;
+ try {
+ element.setAttributeNode(attr);
+ return value;
+ } catch (e) {}
+ // It's a real failure only if setAttribute also fails.
+ return element.setAttribute(name, value, 0);
}
/**
