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

Ignore <base href> in hash history replace #577

Merged
merged 1 commit into from Sep 12, 2019

Conversation

microbouji
Copy link
Contributor

@microbouji microbouji commented Apr 1, 2018

Fixes #574 dealing with createHashHistory's support for the <base> tag. I didn't know how to simulate full-page refresh navigation in the tests but here's how you can manually test it:

Edit index.html to have <base href="/the/base"> in the head and switch to hashHistory. Navigating to the local server without a hash at the end of the url will now redirect you to http://localhost:8080/the/base/#/ in all browsers except for IE where it will redirect to http://localhost:8080/#/

With this change it always behaves as in IE, redirecting to http://localhost:8080/#/ in all browsers.

The related PR #578 changes the output of createHref when a base tag is present.

@mkrauser
Copy link

mkrauser commented Jul 10, 2018

This would also fix an issue we have. We use history for a widget-based app, which is loaded by another website. Those sites load the widgets in various locations. When our widget is loaded, the redirect always leads to {base-href}/# which is not always correct.

Let's say the base-href is "https://some.domain/" and the widget is loaded on page "https://some.domain/test/location" the redirect leads to

https://some.domain/#/

but should lead to

https://some.domain/test/location#/

So please check and merge this. If there's anything I can do to help, just let me know. Thank you very much.

@mwanago
Copy link

mwanago commented Jul 25, 2019

I would very much like to see this one resolved

@mjackson
Copy link
Member

mjackson commented Sep 9, 2019

Why is behaving like IE the desired outcome? Why not make IE behave like all the other browsers?

@microbouji
Copy link
Contributor Author

If I remember correctly this is not about browser inconsistency, but about being able to use HashRouter even when your page has a base tag in its markup (which people are saying might've been put there by another system that you have no control over).

The intent of that replaceHashPath function seems to be to just replace the part of the url after the hash. But when the # is missing and the page has a base tag in it, it redirects to the base url instead.

So, regardless of the underlying bugs and browser inconsistencies, the expectation is that when you navigate to a page with a <base href="whatever"> at the URL:

http://localhost:8080/

it will redirect in all browsers to:

http://localhost:8080/#/

instead of:

http://localhost:8080/whatever/#/

@mjackson
Copy link
Member

Thanks for clarifying, @microbouji :)

IIRC the current code intentionally respects the <base href>. Not saying it should, only that it was a conscious decision.

I was probably thinking about HTML links, and how when you click on them they (usually) do a push, and since links incorporate the <base href> then history.push (and history.replace) should too. But that thinking is conflating two concepts.

Anyway, happy to merge this but it is technically a breaking change, so it'll be released in our next major.

@mjackson mjackson changed the title Fix hash replacement redirect Ignore <base href> in hash history replace Sep 10, 2019
@mjackson
Copy link
Member

Actually, no, I take that back. This is not a breaking change because push already ignores the <base href>. It's really just a bug fix. We can go ahead and release this in a patch version of 4.

@mjackson mjackson mentioned this pull request Sep 12, 2019
25 tasks
@mjackson mjackson merged commit 7324006 into remix-run:master Sep 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IE bug with window.location.replace and base tag
4 participants