Fix OAuth popup being blocked by pop-up blocker in Firefox and IE #537
Conversation
When the user clicked the "Log in" link, the URL of the
"oauth.authorize" endpoint was fetched via an async Promise-returning
method before the `window.open` call was made. This meant that the
`window.open` call did not happen in the turn of the event loop that was
triggered by the user action and so Firefox & IE's popup blockers deemed
the call to have happened outside the context of a user gesture and
prevented the window being opened.
Chrome, Safari & Edge have different heuristics and did not block the
popup before.
Fix the issue by opening the window directly when the user clicks on the
"Log in" button, at a dummy URL ("about:blank"), and then changing the
window's location once the authorization endpoint URL has been fetched.
Fixes #534
Codecov Report
@@ Coverage Diff @@
## master #537 +/- ##
==========================================
+ Coverage 90.93% 91.01% +0.07%
==========================================
Files 136 136
Lines 5426 5529 +103
Branches 945 962 +17
==========================================
+ Hits 4934 5032 +98
- Misses 492 497 +5
Continue to review full report at Codecov.
|
Together with [1] this fixes the OAuth popup failing to appear when clicking "Log in" in IE 11. [1] #537
|
Confirmed that I can reproduce the issue in master and that this fixes it. Also that this branch still works fine in Chrome too |
| top: top, | ||
| width: width, | ||
| height: height, | ||
| }).replace(/&/g, ','); |
There was a problem hiding this comment.
For readability I think it'd be worth moving this into a strWindowFeatures(obj) function, or else just adding a code comment, I had to debug this and look into the window.open docs to figure out what was going on
There was a problem hiding this comment.
I added a comment. This legacy API unfortunately has a somewhat suboptimal interface.
| // user gesture in Firefox & IE 11 and their popup blocking heuristics will | ||
| // prevent the window being opened. See | ||
| // https://github.com/hypothesis/client/issues/534 and | ||
| // https://github.com/hypothesis/client/issues/535. |
There was a problem hiding this comment.
Very good to have this comment here
When the user clicked the "Log in" link, the URL of the
"oauth.authorize" endpoint was fetched via an async Promise-returning
method before the
window.opencall was made. This meant that thewindow.opencall did not happen in the turn of the event loop that wastriggered by the user action and so Firefox & IE's popup blockers deemed
the call to have happened outside the context of a user gesture and
prevented the window being opened.
Chrome, Safari & Edge have different heuristics and did not block the
popup before.
Fix the issue by opening the window directly when the user clicks on the
"Log in" button, at a dummy URL ("about:blank"), and then changing the
window's location once the authorization endpoint URL has been fetched.
Fixes #534
Testing note: OAuth login does not yet work in IE for other reasons which will be fixed separately, so for the moment, just test following the steps in #534.