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

Fix weird error happening when logging in through Facebook on safari … #206

Closed
wants to merge 1 commit into from

Conversation

@thebillkidy
Copy link
Contributor

thebillkidy commented Apr 19, 2016

…browsers, fixes #191

@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Apr 20, 2016

Thanks for the fix, here's a few comments we can address:

  1. Modify this test ✖43) errors on missing cookie in token step so that it tests the new functionality. So verify you get 200 from the first response, then send the query again with the refresh query parameter and ensure you get the 500.
  2. There are linting issues:
/home/travis/build/hapijs/bell/lib/oauth.js:
        Line 200: indent - Expected indentation of 8 space characters but found 0.
        Line 201: indent - Expected indentation of 4 space characters but found 12.
        Line 203: brace-style - Closing curly brace appears on the same line as the subsequent block.
        Line 203: no-else-return - Unexpected 'else' after 'return'.
        Line 204: no-var - Unexpected var, use let or const instead.
        Line 207: indent - Expected indentation of 0 space characters but found 8.

If you need any help with those things, let me know and I can assist.

@ldesplat ldesplat added this to the 7.5.2 milestone Apr 20, 2016
@thebillkidy

This comment has been minimized.

Copy link
Contributor Author

thebillkidy commented Apr 20, 2016

Hi! Thanks for mentioning, I know what you mean but I will be unable to do so for the next month due to some work that I currently have. Are you able to assist with this?

@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Apr 23, 2016

Fixed in #208 . Includes your commit so closing this.

@ldesplat ldesplat closed this Apr 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.