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

Route is not resolved on hash change in IE #1734

Closed
uNmAnNeR opened this issue Mar 23, 2017 · 17 comments · Fixed by #1766
Closed

Route is not resolved on hash change in IE #1734

uNmAnNeR opened this issue Mar 23, 2017 · 17 comments · Fixed by #1766
Assignees
Labels
Type: Bug For bugs and any other unexpected breakage
Milestone

Comments

@uNmAnNeR
Copy link

Hello!

I found that in IE supportsPushState is true, but onpopstate will never be called. So links does not work.

Example here: https://jsfiddle.net/1bj54j8v/

@pygy
Copy link
Member

pygy commented Mar 23, 2017

That's super odd... What IE versions are affected?

Edit: In theory, supportsPushState is false for IE9 and true otherwise, and IE10+versions support onpopstate...

http://caniuse.com/#feat=history

@pygy
Copy link
Member

pygy commented Mar 23, 2017

Could you try this? It is your fiddle, but as a top level page (no iframes added by jsfiddle).

https://bl.ocks.org/pygy/raw/a067feff54d58d36f17bc6851749afe3/#/home

@uNmAnNeR
Copy link
Author

Could you try this? It is your fiddle, but as a top level page (no iframes added by jsfiddle).

Sure. Does not work on IE 11.953.14393.0.

In theory, supportsPushState is false for IE9 and true otherwise, and IE10+versions support onpopstate...

Found this issue for all IE before Edge: http://stackoverflow.com/a/41473506

By the way, mithril 0.2.x worked fine.

@pygy pygy self-assigned this Mar 24, 2017
@pygy pygy added the Type: Bug For bugs and any other unexpected breakage label Mar 24, 2017
@pygy
Copy link
Member

pygy commented Mar 24, 2017

Thanks for the details. I suppose that v0.2 uses onhashchange in hash mode, even when browsers support pushState.

@dead-claudia
Copy link
Member

@pygy It does.

@pygy
Copy link
Member

pygy commented Mar 27, 2017

I gave this a crack this morning with little success (many tests are broken by the change, and the mocks do not handle "debounced async" onhashchange, which doesn't help...)

@pygy pygy closed this as completed in #1766 Apr 1, 2017
@pygy pygy reopened this Apr 3, 2017
@pygy pygy added this to the v1.1.2 milestone Apr 3, 2017
@pygy
Copy link
Member

pygy commented Apr 3, 2017

Sorry, I didn't mean to close this, I didn't know that just mentioning an issue in a commit message closed issues (vs "fix #xxxx" ).

@dead-claudia
Copy link
Member

@pygy That convenience feature is usually useful in GitHub, but yeah, sometimes, it does get annoying.

@pygy
Copy link
Member

pygy commented Jun 18, 2017

I'm afraid the fix for this will be breaking: pygy@bd7230f#diff-08af55052ff6ecc22309ea97e4711428R101

This is impossible to implement for IE9 (and it has never been), but if we want the router to work identically in every env in hash mode, we can't cheat with replaceState() when the routed tree is mounted.

This test would have passed in non-IE browsers...

There's also pygy@bd7230f#diff-08af55052ff6ecc22309ea97e4711428R799 that must be elucidated, it may be a pushStateMock bug or a router bug.

Edit: here's the problem child in full:

if (prefix[0] !== "#") {
	o("default route doesn't break back button", function(done) {
		// impossible to do with onhashchange, "new.com" should equal "old.com"
		init("")
		$window.location.href = "http://old.com"
		$window.location.href = "http://new.com"

		route(root, "/a", {
			"/a" : {
				view: function() {
					return m("div")
				}
			}
		})

		callAsync(function() {
			o(root.firstChild.nodeName).equals("DIV")

			o(route.get()).equals("/a")

			$window.history.back()

			o($window.location.pathname).equals("/")
			o($window.location.hostname).equals("old.com")

			done()
		})
	})
}

Before this, onhashchange-based routing wasn't tested at all :-/

@dead-claudia
Copy link
Member

@pygy That's concerning... 😟

@pygy
Copy link
Member

pygy commented Jun 21, 2017

@isiahmeadows Worse, I recently realised that onhashchange wasn't debounced as I thought it was due to a JSBin bug(jsbin/jsbin#3078) (and not enough cross checking on my hand)...

@pygy pygy modified the milestones: v1.1.2, 2.0.0 Jul 5, 2017
@andraaspar
Copy link
Contributor

I ran into this recently and had to create a workaround. Just in case it helps anyone.

@dead-claudia
Copy link
Member

I'm punting this to post-v2, but I might be able to fix it in a future patch release. Specifically, we could detect IE and see what we can do to work around the popstate bug, but it's not exactly going to be easy to do.

If someone beats me to it, I'll gladly consider merging it. 🙂

@barneycarroll
Copy link
Member

This ticket is 3 years old, and IE is no longer a support target. I'm closing this because with the passing of time, this no longer constitutes a bug.

@dead-claudia
Copy link
Member

@barneycarroll This needs checked against IE11 - MS still supports it: https://docs.microsoft.com/en-US/lifecycle/faq/internet-explorer-microsoft-edge

I say we should put the lid on it August 17 of this year, aligning with Microsoft 365: https://techcommunity.microsoft.com/t5/microsoft-365-blog/microsoft-365-apps-say-farewell-to-internet-explorer-11-and/ba-p/1591666#ftag=MSF278e2c0

@barneycarroll
Copy link
Member

Excellent. Given our resources I say we just drop support now. There is no sense leaving open the window of hope and opportunity for a browser that is going to be discontinued in half a year.

@dead-claudia
Copy link
Member

Sure, let's do that. Just not this immediate next release. How does that sound?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug For bugs and any other unexpected breakage
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

5 participants