Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Ember.get returns 'length' of null #3760

Closed
jkintscher opened this issue Nov 19, 2013 · 1 comment · Fixed by #3852
Closed

[BUGFIX] Ember.get returns 'length' of null #3760

jkintscher opened this issue Nov 19, 2013 · 1 comment · Fixed by #3852
Labels

Comments

@jkintscher
Copy link

I noticed this first when looking into a problem I was having with Ember Model which would break on hasMany relations for no apparent reason. This is the related issue.

When the first parameter that’s passed to Ember.get is null and the keyName is just a toplevel path the lookup is immediately relayed to Ember._getPath which finds the property on Ember.lookup or rather window. That’s helpful in a lot of cases but can also return unexpected values as it did in the above situation.

var foo = null;
Ember.get(foo, 'length'); // >= 0

Depending on the number of frames on the page window.length in this case returns an integer that’s always >= 0. And even if your application isn’t using frames itself any extension (like the Readability Chrome extension for me) can inject <iframe>s into the page which increases that number.

I’m not sure if this is anything you’d want to fix directly in the framework but let me know if you do!
Either way I wanted to bring this up as I didn’t find any existing Issue for it and spent some time tracking down the source—which hopefully someone else looking into the same problem can save now.

@rwjblue
Copy link
Member

rwjblue commented Dec 22, 2013

Closing in favor of PR: #3852.

@rwjblue rwjblue closed this as completed Dec 22, 2013
mixonic added a commit to mixonic/ember.js that referenced this issue Nov 24, 2014
* Changes globals to only mean properties starting with $ or a capital letter.
* Changes normalizeTuple(obj, 'Foo') to resolve Foo as a global. This
  makes the behavior consistent with normalizeTuple(obj, 'Foo.bar') which
  already resolves Foo as a global (and looks up bar).
* Changes normalizeTuple(null, 'Foo') to resolve Foo as a global.
* Specs normalizeTuple('null', 'foo') to resolve to [undefined, '']. This
  is unfortunate but needed to keep getPath happy.

This code is old, and tricky to modify due to perf optimizations and
edge case behavior. It would be great to refactor more heavily at some
other date. I've spec'd out more of the current behavior and edge cases
in this commit.

Fixes emberjs#3760
mixonic added a commit to mixonic/ember.js that referenced this issue Mar 8, 2015
* Changes globals to only mean properties starting with $ or a capital letter.
* Changes normalizeTuple(obj, 'Foo') to resolve Foo as a global. This
  makes the behavior consistent with normalizeTuple(obj, 'Foo.bar') which
  already resolves Foo as a global (and looks up bar).
* Changes normalizeTuple(null, 'Foo') to resolve Foo as a global.
* Specs normalizeTuple('null', 'foo') to resolve to [undefined, '']. This
  is unfortunate but needed to keep getPath happy.

This code is old, and tricky to modify due to perf optimizations and
edge case behavior. It would be great to refactor more heavily at some
other date. I've spec'd out more of the current behavior and edge cases
in this commit.

Fixes emberjs#3760
mixonic added a commit that referenced this issue Mar 11, 2015
* Changes globals to only mean properties starting with $ or a capital letter.
* Changes normalizeTuple(obj, 'Foo') to resolve Foo as a global. This
  makes the behavior consistent with normalizeTuple(obj, 'Foo.bar') which
  already resolves Foo as a global (and looks up bar).
* Changes normalizeTuple(null, 'Foo') to resolve Foo as a global.
* Specs normalizeTuple('null', 'foo') to resolve to [undefined, '']. This
  is unfortunate but needed to keep getPath happy.

This code is old, and tricky to modify due to perf optimizations and
edge case behavior. It would be great to refactor more heavily at some
other date. I've spec'd out more of the current behavior and edge cases
in this commit.

Fixes #3760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants