Potential usage of reserved keywords in Parser.getterFn

[zenorbi]

At the line 928:
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key + ';\n';

should be replaced to
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '["' + key + '"];\n';

as the key can potential be a reserved keyword like float, class, for. I know about 1 instance where the key is "in" which is part of the for (k in o) syntax.

[gkalpak]

I don't think there is a problem with accessing a property using the dot notation even if that property's name is a reserved keyword (but I might be missing something).

@zenorbi, do you have an example expression that causes an error or unexpected behaviour due to this ?

[zenorbi]

In IE8 it does throw an exception. I know that IE8 doesn't participate on automated testing, but this is only a 4 character modification.

[Narretz]

We don't support angular in 1.3.x. Does it happen in 1.2.x?

[zenorbi]

It doesn't happen there as the key is wrapped in square brackets.

[gkalpak]

@zenorbi: Could you post the breaking example. I am just curious and I wasn't able to reproduce it.

[zenorbi]

I've been able to narrow it down to this:
<div ng-class="{in: animate}"></div>

A div like this gets inserted by angular-ui and causes the parser to try and parse the "in" part.

[btford]

@zenorbi if you have a reproduction for Angular 1.2.x that is broken in IE8, we can take a look at it.

[zenorbi]

@btford Angular 1.2.x doesn't have this bug, as that uses square brackets. This is the line from angular 1.2.25:
: '((k&&k.hasOwnProperty("' + key + '"))?k:s)') + '["' + key + '"]' + ';\n' +
which is correct, as this will translate to (...)["in"] instead of what 1.3 would do (...).in.

[caitp]

that sounds like a pretty simple fix then, did we change that for perf
reasons?

On Wed, Sep 17, 2014 at 1:59 PM, Zenorbi notifications@github.com wrote:

@btford https://github.com/btford Angular 1.2.x doesn't have this bug,
as that uses square brackets. This is the line from angular 1.2.25:
: '((k&&k.hasOwnProperty("' + key + '"))?k:s)') + '["' + key + '"]' +
';\n' +
which is correct, as this will translate to (...)["in"] instead of what
1.3 would do (...).in.

—
Reply to this email directly or view it on GitHub
#9131 (comment).

[zenorbi]

I know that most of the stuff doesn't actually work in IE8 like Object.getPrototypeOf or Object.create but I was able to shim that in along with a bunch of other stuff. Currently this is the only thing which needs modification on the angular.js file. I can just keep replacing this line in each angular version, I'm just hoping that because this is such a small adjustment, it can live in the real source.

[caitp]

if we had a good reason to change it, then it probably can't. But lets find out. If you want you could submit a fix for it, it should be a pretty trivial PR

[lgalfaso]

@caitp it was done for performance reasons c54228f