Navigation Link: Compare internal links by host instead of origin#76015
Merged
Navigation Link: Compare internal links by host instead of origin#76015
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Determine internal vs external by
new URL('https://mysite.com').hostinstead ofnew URL('https://mysite.com').origin. On trunk, when you use the same URL but change the http:// vs https://, one is considered external.Why?
When determining whether a navigation link is internal or external, the previous implementation compared URLs by their
origin(scheme + host + port). This caused links usinghttp://to be incorrectly classified as external when the site URL usedhttps://, even if they pointed to the same domain.How?
This PR fixes the comparison to use
hostinstead oforigin, sohttp://example.com/pageandhttps://example.com/pageare both treated as internal links to anhttps://example.comsite.Additionally, the
window.location.originfallback (used whenhomeUrlwas not provided) has been removed. ThehomeUrlis now required for internal/external detection; without it, URL parsing fails gracefully and the link is treated as external.A unit test covering the http/https case is included.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast