Skip to content

Commit

Permalink
Added a try..catch to the cookies test
Browse files Browse the repository at this point in the history
  • Loading branch information
stucox committed Nov 29, 2013
1 parent 6d7c5ff commit f346166
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions feature-detects/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,18 @@ define(['Modernizr'], function( Modernizr ) {
Modernizr.addTest('cookies', function () {
// navigator.cookieEnabled is in IE9 but always true. Don't rely on it.

// Create cookie
document.cookie = "cookietest=1";
var ret = document.cookie.indexOf("cookietest=") != -1;
// Delete cookie
document.cookie = "cookietest=1; expires=Thu, 01-Jan-1970 00:00:01 GMT";
return ret;
// try..catch because some envs expose `document.cookie` but just error
// if you try to use it
try {
// Create cookie
document.cookie = "cookietest=1";
var ret = document.cookie.indexOf("cookietest=") != -1;
// Delete cookie
document.cookie = "cookietest=1; expires=Thu, 01-Jan-1970 00:00:01 GMT";
return ret;
}
catch (e) {
return false;
}
});
});

7 comments on commit f346166

@patrickkettner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what envs?

@stucox
Copy link
Member Author

@stucox stucox commented on f346166 Dec 1, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I’m hacking around on data: URLs, mostly :-)

But I believe Chrome (maybe others) will throw an error if you try to access document.cookie in a sandboxed iframe without the allow-same-origin flag too.

@stucox
Copy link
Member Author

@stucox stucox commented on f346166 Dec 1, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want me to make my comment more explicit?

@patrickkettner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, cool. less certain envs than certain situations, then, right? (Envs, to me, means browers)

@patrickkettner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to ad an e.g

@patrickkettner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

er, more importantly, I think it is good to note its a securityerror, rather than a bug

@stucox
Copy link
Member Author

@stucox stucox commented on f346166 Dec 1, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.