0.14 is broken in IE11

[mattlewis92]

It looks like the changes between 0.13 and 0.14 broke this library in IE11, after signing in ok to the popup window it redirects back to the app and never closes the popup + continues the process of adding the account. Rolling back from 0.14 to 0.13.4 fixes the problem.

[sahat]

After looking into it further, seems like IE is not normalizing URL pathname to "/", like other browsers. I will try to release a patch soon that fixes this issue.

[mattlewis92]

Fantastic, thanks for the quick response!

On 1 Mar 2016, at 20:18, Sahat Yalkabov notifications@github.com wrote:

After looking into it further, seems like IE is not normalizing URL pathname to "/", like other browsers. I will try to release a patch soon that fixes this issue.

—
Reply to this email directly or view it on GitHub.

[sahat]

Try the latest satellizer.js file, if that fixes your problem with IE, I will bump the version number and publish it on NPM.

[mattlewis92]

I gave your fix a try and it doesn't seem to work, would you like me to try and put together a minimally failing example?

[sahat]

Sure, but I just find it strange that didn't work for you. I was able to reproduce the problem in IE11 and pretty certain it is related to URL pathname mismatch.

Do you have Protected Mode turned off in IE options?

[mattlewis92]

I don't think I have protected mode enabled, I'm just using the IE VM from the modern.ie site. How can I double check? I'll put together an example tomorrow for you.

[sahat]

By default it is turned on. The issue you are describing occurs only when running the app locally. If you are to deploy it first, to Heroku for example, then visit that URL from your IE VM, it should work. If it doesn't, then it is likely another issue.

[sahat]

Hmm you did say it works with 0.13.4, so maybe I was wrong about Protected Mode issue.

[mattlewis92]

Yup it's deployed on a proper domain with ssl and it's still happening

[LuisMejiaF]

I'm experiencing the same issue when trying with authenticate with Google or Facebook, but I'm using Chrome: Satellizer opens the popup but it remains open and nothing happens.

I noticed that the URL has the code parameter, but the structure is such that angular cannot find the route.

For example: my application redirects on authentication callback to mysite.com/#/, which is the homepage. But satellizer facebook or google somehow redirect to mysite.com/?code=xxxxxx/#/, and in that case angular cannot find the route.

I'm using Chrome version 48.0.2564.116 in windows desktop.

[mattlewis92]

I dug into this a bit more today and have found the cause of the problem.

I put a debug statement here with console.log('DEBUG', popupWindowPath, redirectUriPath);

As you can see popupWindowPath doesn't have the port number and redirectUriPath has the port number so the popup never closes.

[sahat]

Ah, yet another IE's weird behavior. Both Chrome and Firefox return empty port number. I am not sure if it's the best way, but I could explicitly ignore port number concatenation if port === "80" or port === "443" (http and https ports, respectively).

These minor differences, like pathname is empty string in IE and slash in Chrome/Firefox, or empty port number in Chrome/Firefox and port 80 in Internet Explorer stem from this code:

var redirectUriParser = document.createElement('a');
redirectUriParser.href = redirectUri;

It is necessary to parse redirectUri so issues like popup not closing wouldn't occur if redirectUri is missing a slash at the end, for example http://localhost:3000 !== http://localhost:3000/.

As pointed out in earlier issues, I can't make it check just the hostname, because that will fail for the case when OAuth authorization server is on the same domain. Can't use just the pathname either, because most redirect urls require absolute path. So, the only option is to parse both URLs and check individual URL bits, like it is doing right now.

[mattlewis92]

What about going the other way round and defining the port number to 80 or 443 if not set and the protocol is http / https respectively?

[sahat]

Is there any advantage to doing so? In either case I will have about the same amount of work: 1) not using a port number if port is 80 or 443, or 2) using port number 80 if http, 443 if https, or use existing port number if available.

[mattlewis92]

Normalising the data just seems like a cleaner implementation to me, but either would work fine.

[sahat]

Alright, while I am writing unit tests for this function, could you provide me some URLs to test potential edge cases? I want to solve this problem once and for all.

[mattlewis92]

Sure thing, just to be sure we're on the same page do you mean this function?

As a side note, have you ever tried browserstack before? It's free for open source and you can have travis run all your tests in IE9-11, FF etc to catch browser differences like this in the future. It's super easy to integrate as you're already using karma, just a few extra bits added to your config

[sahat]

Yes that's the function getFullUrlPath.

I have heard of BrowserStack but did not realize they offer a free tier for open-source projects. I'll check them out. It's a pain testing all browsers with every release.

[mattlewis92]

It looks like you have to apply, but I doubt they'd say no for a project this popular.

For test cases, extracted from running this in each browsers console:

var a = document.createElement('a'); a.href='http://google.com/test'; console.log(a.protocol, a.hostname, a.port, a.pathname);

Chrome, Safari, FF, domain http://google.com/test

{
  protocol: 'http:',
  hostname: 'google.com',
  port: '',
  pathname: '/test'
}

Chrome, Safari, FF, domain https://google.com/test

{
  protocol: 'https:',
  hostname: 'google.com',
  port: '',
  pathname: '/test'
}

IE 9-11, domain http://google.com/test

{
  protocol: 'http:',
  hostname: 'google.com',
  port: '80',
  pathname: 'test'
}

IE 9-11, domain https://google.com/test

{
  protocol: 'https:',
  hostname: 'google.com',
  port: '443',
  pathname: 'test'
}

[sahat]

Thanks. Alright, try satellizer.js from master branch again. It should normalize the port number now.

[LuisMejiaF]

It is fixed for me in Chrome Desktop with the master branch file, but it fails in Mobile browers in Android, I get the same behavior that I got in Chrome earlier.

[mattlewis92]

@sahat the fixes in master are working perfectly for me now in IE11, thanks for that!

@LuisMejiaF I checked the output of var a = document.createElement('a'); a.href='http://google.com/test'; console.log(a.protocol, a.hostname, a.port, a.pathname); in an android VM and it gave the same output as chrome. It's either a different bug, or a discrepancy in your specific android browser (which are well known for varying wildly in implementations, hence the existence of crosswalk etc)

[sahat]

@mattlewis92 Excellent. Thanks for your help too. I will push a new version soon.

[mattlewis92]

Awesome, thanks!

On 4 Mar 2016, at 23:12, Sahat Yalkabov notifications@github.com wrote:

@mattlewis92 Excellent. Thanks for your help too. I will push a new version soon.

—
Reply to this email directly or view it on GitHub.

[fatcop]

any new tagged version for this fix imminent ?

[barryvdh]

I've run into this issue as well. Seems to be present in the latest release, but the current dev-master is working. I've installed bower install "https://github.com/sahat/satellizer.git#ae7912b8e100a0ec86f66526c0b342b5392189b3" for now, but a tag would be much appreciated!

[clarle]

Fixed in 0.14.1.

Let us know if you see any other issues, and thanks for reporting!