Skip to content

Normalize the way to get the URL in case that path is not provided.#310

Open
jorgecasar wants to merge 2 commits into
visionmedia:masterfrom
jorgecasar:unify-default-url-generation
Open

Normalize the way to get the URL in case that path is not provided.#310
jorgecasar wants to merge 2 commits into
visionmedia:masterfrom
jorgecasar:unify-default-url-generation

Conversation

@jorgecasar
Copy link
Copy Markdown
Contributor

This solve the error that happens when application runs in root path (location.pathname === '/') and navigate using anchor link. There was 2 diferent ways to generate the URL, one for page.start and other for cases when event.state is not provided on popstate event.

@froucher
Copy link
Copy Markdown

froucher commented Sep 7, 2015

Great! thx @jorgecasar! Please, this pull request is urgent for us...

@jorgecasar jorgecasar force-pushed the unify-default-url-generation branch from 5423b1e to 4d18aec Compare September 11, 2015 11:41
@jorgecasar jorgecasar force-pushed the unify-default-url-generation branch from 4d18aec to ba30c47 Compare October 8, 2015 08:19
@matthewp
Copy link
Copy Markdown
Collaborator

Can you explain what this fixes?

@jorgecasar
Copy link
Copy Markdown
Contributor Author

jorgecasar commented Jan 21, 2018

There are 3 places in the code where the path is managed. And each part get it with different approach. This commit try to normalize and always get the path following the same rules. Sometimes you lost url info because of the parsers. The correct one is the Approach 1, the other cause errors because of the search info and hashbang.

Approach 1:
var url = (hashbang && ~location.hash.indexOf('#!')) ? location.hash.substr(2) + location.search : location.pathname + location.search + location.hash;

Approach 2:
var url = location.pathname + location.hash

Approach 3:
var path = el.pathname + el.search + (el.hash || '');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants