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

Issue #20 - Items cannot be added to cart if .NET incorrectly detects browser cookie capability #25

Merged
merged 3 commits into from
Jul 13, 2017

Conversation

MarkXA
Copy link
Contributor

@MarkXA MarkXA commented Jun 26, 2017

Fixes #20.

Removed unnecessary and potentially damaging browser capability checks. Browsers erroneously detected as not supporting cookies could not retain shopping cart state.

@WillStrohl
Copy link
Member

These updates assume that (1) the request context always won't be null; and (2) cookies are always supported. While I understand why they're implemented the way they are, isn't there a higher chance of Null Reference Exceptions on both the request and cookie objects with this?

@WillStrohl WillStrohl requested a review from sbwalker June 26, 2017 21:54
@WillStrohl WillStrohl self-assigned this Jun 26, 2017
@MarkXA
Copy link
Contributor Author

MarkXA commented Jun 26, 2017

The HttpContext could potentially be null, but that's already checked. The Request object will always be initialised by the point in the lifecycle at which we're accessing it, and the Cookies collection is also non-null - if no Cookie headers were received, it will simply be empty.

P.S. We have this code tested and running in production :)

@WillStrohl WillStrohl merged commit f400dab into HotcakesCommerce:development Jul 13, 2017
@WillStrohl WillStrohl added this to the 03.00.01 milestone Jul 13, 2017
@MarkXA MarkXA deleted the cookie-fix branch July 13, 2017 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants