Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Do not unconditionally add "//" to absolute URLs and handle authority-less URLs #7263

Closed

Conversation

gabrielschulhof
Copy link

Navigation: Test authority-less protocols with path.getLocation()
Navigation: Do not assume "//" is always part of an absolute URL
Navigation: Expect a trailing slash when testing path.getLocation()

Note: this does indeed represent a fix for gh-6574, but only once a version of
Cordova sporting apache/cordova-wp8#30 is released.

Closes gh-6597
Fixes gh-6574
Fixes gh-6599

// Always use our own URL parser, even though location potentially provides all the
// fields we may need later on. This way, URL parsing is consistent, and we only
// grab location.href from the browser.
var uri = this.parseUrl( url || location.href ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please provide examples where the built in parser is inconsistent or incorrect? Those should be in the comment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, TBH, I think I was thinking of #6810 at the time, but we can't reproduce that one. If the location.host is empty on Android 4.4, then we're better of never relying on it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... so, I'm not sure if that counts as an example.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I've found another reason to do our own parsing. location does not provide the doubleSlash property. Our parser does provide this property, and now that we're not unconditionally appending the double slash, we need it below.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.25%) when pulling 0cb8fc6 on 6574-special-case-x-wmapp-scheme-via-path into 6b46425 on master.

@arschmitz
Copy link
Contributor

👍

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.

Navigation with authority-less URI schemes fail AJAX navigation not working on Windows phone
4 participants