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.

Sign up for GitHub

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

Jump to bottom

Update documentation on the rationale for ignoring cookiesEnabled #1356

Merged
merged 1 commit into from
Jun 13, 2014
Merged

Update documentation on the rationale for ignoring cookiesEnabled #1356

merged 1 commit into from
Jun 13, 2014

Conversation

dbow
Copy link
Contributor

@dbow dbow commented May 22, 2014

In this issue it was reported that navigator.cookiesEnabled always returns true in IE9 but I did not find that to be the case

screen shot 2014-05-22 at 12 04 37 pm

To get it to return false I updated the cookie settings like so:
screen shot 2014-05-22 at 12 05 02 pm

However, I still think cookiesEnabled is unreliable since it does not do a good job of detecting nuanced cookie settings (first party vs third party, site-specific exceptions) reliably. For example, this setting in IE9, which for all intents and purposes is blocking cookies, returns true for cookiesEnabled

screen shot 2014-05-22 at 12 06 37 pm

Just wanted to clarify the rationale in the comment so it's more accurate as to why we ignore this property. If anyone wants to re-word this, feel free.

Full Disclosure: These tests were done in one of the Windows 7 IE9 VMs that Microsoft distributes

@dbow
Copy link
Contributor Author

dbow commented May 22, 2014

Now that I think about it, it seems like we should use navigator.cookieEnabled if it's false - there don't seem to be false negatives now that it sounds like the site-specific exception issue is fixed. That should probably be confirmed. But if it's true, the logic could be

if ('cookieEnabled' in navigator && !navigator.cookieEnabled) {
  return false;
}
// Otherwise do the cookie test.

Will look into this a bit more.

@dbow
Copy link
Contributor Author

dbow commented May 23, 2014

Checked navigator.cookieEnabled with various cookie configurations in Chrome/Firefox/Safari/Opera on Mac and Chrome/Firefox/IE9/IE10 on Windows 7. No surprises from what's already been outlined. Given the fact that there were false negatives in the past, it probably doesn't make sense to use navigator.cookieEnabled if it's false just yet. But I do think the comment should be updated to reflect the actual issues with this attribute as originally outlined.

@patrickkettner
Copy link
Member

Seems very reasonable.

thanks a lot for the PR, @dbow - doc's always need some lovin' :D

patrickkettner added a commit that referenced this pull request Jun 13, 2014
@patrickkettner
Merge pull request #1356 from dbow/cookies
ec7e355 
Update documentation on the rationale for ignoring cookiesEnabled
@patrickkettner patrickkettner merged commit ec7e355 into Modernizr:master Jun 13, 2014
patrickkettner added a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015
@patrickkettner
Merge pull request Modernizr#1356 from dbow/cookies
ba1e67e 
Update documentation on the rationale for ignoring cookiesEnabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dbow @patrickkettner