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

Ie fix #8

Merged
merged 3 commits into from
Aug 9, 2013
Merged

Ie fix #8

merged 3 commits into from
Aug 9, 2013

Conversation

dcendents
Copy link
Contributor

I'm developing a web site with angular and wanted to start using the local storage and came across ngStorage. It works like a charm on Chrome and Firefox but it came out with a strange error on Internet Explorer (Invalid Argument).

Turns out IE9 does not like the method key(0) when the local storage is empty and throws an exception instead of returning null as per the specification.

Changing the for loop to look at the localStorage.length instead fixes the problem.

…rror instead of returning null when the specified key does not exist.
@gsklee
Copy link
Owner

gsklee commented Aug 8, 2013

How about for (var i = 0, k; webStorage.length && (k = webStorage.key(i)); i++) {? ;-)

@dcendents
Copy link
Contributor Author

I think my assessment was wrong, it is not just when the local storage is empty but whenever we try to access a key index that does not exists.

So it needs to validate i is less than the length. You seems to like the compact notation to assign k in the for loop so I'll change it to the following which does work in IE9:

for (var i = 0, k; i < webStorage.length && (k = webStorage.key(i)); i++) {

Hope that's fine with you.

gsklee pushed a commit that referenced this pull request Aug 9, 2013
Bugfix for IE9 where accessing a Web Storage key index that doesn't exist will cause an error
@gsklee gsklee merged commit a4a5fdc into gsklee:master Aug 9, 2013
@gsklee
Copy link
Owner

gsklee commented Aug 9, 2013

Very cool, thanks 👍

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

Successfully merging this pull request may close these issues.

2 participants