fix IE[67] issues with getAttribute/setAttribute

in IE[67] and IE8 in compatibility mode,
domita_test fails in a couple ways.

the problem is el.getAttribute(x) is broken on IE[67].
if you have
  <form id="f" foo="x"><input name="foo"></form>
then f.getAttribute('foo') will return the input node,
instead of 'x'.

setAttribute() is also broken in the same way.

this screws up a lot of things on IE[67].
for example, if a gadget has
      <form id="x"><input name="id"></form>
then html-emitter does not set the form id correctly.

the fix is to use getAttributeNode instead.

I've run domita_test.html manually on
 chrome-4.0.221.8   1 fail (unrelated)
 firefox-3.0.14     0 fail
 firefox-3.5.3      0 fail
 ie-6               7 fail (unrelated)
 ie-8-compat        7 fail (unrelated)
 ie-8-std           9 fail (unrelated)
 opera-10.00        7 fail (unrelated)
 safari-4.0.3       1 fail (unrelated)

safari-3.2.1 and opera-9.64 completely fail to run domita_test,
not sure why yet.

[MikeSamuel, 2009-10-14 03:10:27]

http://codereview.appspot.com/129045/diff/1/5
File src/com/google/caja/plugin/bridal.js (right):

http://codereview.appspot.com/129045/diff/1/5#newcode391
Line 391: element.setAttributeNode(node);
So this works for class and for?

http://codereview.appspot.com/129045/diff/1/2
File tests/com/google/caja/plugin/domita_test_untrusted.html (right):

http://codereview.appspot.com/129045/diff/1/2#newcode2050
Line 2050: assertEquals('testcontainer inline', el.getAttribute('class'));
Can we add a local function assertClassIs(el, className) that tests both
className and the attribute?

http://codereview.appspot.com/129045/diff/1/2#newcode2662
Line 2662: if (attrNode == null || !attrNode.specified) {
indentation

http://codereview.appspot.com/129045/diff/1/2#newcode2810
Line 2810: jsunitRegister('testForAttribute', function testBug1066() {
Maybe change the function name too.

http://codereview.appspot.com/129045/diff/1/6
File third_party/js/jqueryjs/runtest/env.js (right):

http://codereview.appspot.com/129045/diff/1/6#newcode523
Line 523: null;
Any reason not to expose the underlying Attr?

[felix8a, 2009-10-15 18:36:15]

updated snapshot

http://codereview.appspot.com/129045/diff/1/5
File src/com/google/caja/plugin/bridal.js (right):

http://codereview.appspot.com/129045/diff/1/5#newcode391
Line 391: element.setAttributeNode(node);
On 2009/10/14 03:10:27, MikeSamuel wrote:
> So this works for class and for?

yeah, it works on all the A-grade browsers

http://codereview.appspot.com/129045/diff/1/2
File tests/com/google/caja/plugin/domita_test_untrusted.html (right):

http://codereview.appspot.com/129045/diff/1/2#newcode2050
Line 2050: assertEquals('testcontainer inline', el.getAttribute('class'));
On 2009/10/14 03:10:27, MikeSamuel wrote:
> Can we add a local function assertClassIs(el, className) that tests both
> className and the attribute?

ok

http://codereview.appspot.com/129045/diff/1/2#newcode2662
Line 2662: if (attrNode == null || !attrNode.specified) {
On 2009/10/14 03:10:27, MikeSamuel wrote:
> indentation

ok

http://codereview.appspot.com/129045/diff/1/2#newcode2810
Line 2810: jsunitRegister('testForAttribute', function testBug1066() {
On 2009/10/14 03:10:27, MikeSamuel wrote:
> Maybe change the function name too.

ah, yeah.  typo.

http://codereview.appspot.com/129045/diff/1/6
File third_party/js/jqueryjs/runtest/env.js (right):

http://codereview.appspot.com/129045/diff/1/6#newcode523
Line 523: null;
On 2009/10/14 03:10:27, MikeSamuel wrote:
> Any reason not to expose the underlying Attr?

ah, I didn't think of that.  yeah, exposing the Attr is better and seems to
work.

[MikeSamuel, 2009-10-15 19:37:37]

LGTM

http://codereview.appspot.com/129045/diff/3001/1012
File third_party/js/jqueryjs/runtest/env.js (right):

http://codereview.appspot.com/129045/diff/3001/1012#newcode520
Line 520: this._dom.getAttributeNode(name) : null;
I think
    return this._dom.getAttributeNode(name)
would be identical to this.

[felix8a, 2009-10-15 19:54:14]

http://codereview.appspot.com/129045/diff/3001/1012
File third_party/js/jqueryjs/runtest/env.js (right):

http://codereview.appspot.com/129045/diff/3001/1012#newcode520
Line 520: this._dom.getAttributeNode(name) : null;
On 2009/10/15 19:37:37, MikeSamuel wrote:
> I think
>     return this._dom.getAttributeNode(name)
> would be identical to this.

ah, yeah it is.  ok, going to commit with that change, in a moment.

[felix8a, 2009-10-15 19:57:38]

commited at http://code.google.com/p/google-caja/source/detail?r=3801