jQuery 1.9.0 fails in both ES5 and ES5/3

[GoogleCodeExporter]

In the playground, r5238, the following code:

  <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.9.0/jquery.min.js"
      type="text/javascript">
  </script>
  <div>foo</div>

causes in ES5/3:

  Uncaught script error: 'Cannot read property 'expando_v___' of undefined' in source: 'jquery.min.js' at line: 1
  jquery.min.js:1: Cannot read property 'expando_v___' of undefined
  Rejecting complex event handler div onsubmit="t" es53-taming-frame.js:6401
  Rejecting <div>.setAttribute( onsubmit , t ) es53-taming-frame.js:6401

and in ES5:

  Uncaught script error: 'TypeError: Cannot read property 'expando' of undefined' in source:
      'http://ajax.googleapis.com/ajax/libs/jquery/1.9.0/jquery.min.js' at line: 1

The jQuery file itself:

  http://ajax.googleapis.com/ajax/libs/jquery/1.9.0/jquery.js

contains some code that says:

  // Support: IE<9 (lack submit/change bubble), Firefox 17+ (lack focusin event)
  // Beware of CSP restrictions (https://developer.mozilla.org/en/Security/CSP), test/csp.php
  for ( i in { submit: true, change: true, focusin: true }) {
    div.setAttribute( eventName = "on" + i, "t" );
    support[ i + "Bubbles" ] = eventName in window || div.attributes[ eventName ].expando === false;
  }

so we are rejecting the setting of the attribute "onsubmit = t", then 
div.attributes.onsubmit is undefined, then we get the error.

Original issue reported on code.google.com by ihab.a...@gmail.com on 6 Feb 2013 at 7:18

[GoogleCodeExporter]

More details about this.

Note this line in Domado: 
http://code.google.com/p/google-caja/source/browse/trunk/src/com/google/caja/plu
gin/domado.js?r=5249#1979

What we are doing is checking that attributes of type html4.atype.SCRIPT match 
the SIMPLE_HANDLER_PATTERN. This is something of an artifact of ES5/3, where we 
were unable to cajole the handler JavaScript on the fly. In ES5, we can -- but 
we haven't modified Domado yet to do so.

There is no variable 't' in the top lexical scope of how we evaluate scripts, 
so this would probably throw if we try to do it naively.

Original comment by ihab.a...@gmail.com on 6 Feb 2013 at 7:34

[GoogleCodeExporter]

Throwing should only occur when the handler is actually executed, so that isn't 
a problem. I think that it would be OK to, instead of rejecting the 
setAttribute effect, generating a corresponding function which doesn't actually 
do anything.

We do support arbitrary non-SIMPLE_HANDLERs in Domado under some circumstance, 
but evidently not this one. I'd like to take a look at it.

Also note that issue 1483 will require some rewiring of the 
SIMPLE_HANDLER_PATTERN stuff which might get along well with this.

Original comment by kpreid.switchb.org on 6 Feb 2013 at 2:30

[GoogleCodeExporter]

Ihab: we *should already* have support for arbitrary handlers in this case. See 
line 1970 above. So, this should *not* be a problem in ES5 mode — if it's 
convenient since you're already debugging this, could you find out why it is?

I don't understand in the original code how this 'expando' property of an 
attribute node is supposed to appear (without patching DOM prototypes which we 
don't permit), but even if that fails, there ought to be an attribute node in 
.attributes, not undefined, given that we did not reject the setAttribute (as 
the console suggests we did not).

 The workaround I proposed in comment 2 is still applicable for ES5/3.

Original comment by kpreid.switchb.org on 6 Feb 2013 at 5:00

[GoogleCodeExporter]

Found it. We just don't support .attributes[attributeName]; it's tamed as a 
plain NodeList, not a NamedNodeMap.

I have a patch for the rejected-handler component coming up soon.

Original comment by kpreid.switchb.org on 6 Feb 2013 at 6:04

[GoogleCodeExporter]

Patch for handlers: <https://codereview.appspot.com/7309056>. I will now look 
into .attributes.

Original comment by kpreid.switchb.org on 6 Feb 2013 at 6:26

[GoogleCodeExporter]

.attributes is tricky. We can't support it in the general case without proxies, 
but we can put in a hook for attributes that aren't dynamically added, so a 
sufficiently-good implementation should be doable, but I don't think I'll get 
to it this week.

If we need to support this ASAP, then the thing for someone else to do would be 
a one-off defineProperties inside the getter for 'attributes' which adds JS 
properties for the three attributes that jQuery is looking at here.

Original comment by kpreid.switchb.org on 6 Feb 2013 at 9:16

[GoogleCodeExporter]

Original comment by kpreid.switchb.org on 8 Feb 2013 at 12:45

• Changed state: Started

[GoogleCodeExporter]

Patch for .attributes: <https://codereview.appspot.com/7310064>. With it, the 
code sample from jQuery does not crash. I have not tested loading all of jQuery 
1.9.0.

Original comment by kpreid.switchb.org on 8 Feb 2013 at 1:53

• Changed state: Pending

[GoogleCodeExporter]

Aforementioned patch is in as r5307. Verified that the original test case 
successfully loads jQuery in the playground (in Chrome) as of this revision.

Original comment by kpreid.switchb.org on 28 Feb 2013 at 8:11

• Changed state: Fixed