Skip to content

Fix error when using head base-href #3256

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

Merged
merged 1 commit into from
Dec 4, 2022

Conversation

parlough
Copy link
Member

@parlough parlough commented Dec 4, 2022

If data-using-base-href is true, that means it is using the base element to set href, not that it is using a href attribute on the body element. To support using the href specified there, we also need to properly use an actual relative path instead of basing it on the full window.location.href.

Fixes #3172

@parlough parlough marked this pull request as draft December 4, 2022 06:03
@parlough parlough changed the title Fix error when use head base-href Fix error when using head base-href Dec 4, 2022
@parlough parlough force-pushed the fix/error-when-using-base-href branch from c30a8df to 4119fba Compare December 4, 2022 07:17
@parlough parlough marked this pull request as ready for review December 4, 2022 07:18
@srawlins
Copy link
Member

srawlins commented Dec 4, 2022

This base href stuff freaks me out. It affects all links, so requires a lot of test coverage, which we don't have, combinatorically speaking. :/ Also we don't have web tests. #3027

Copy link
Member

@srawlins srawlins left a comment

Choose a reason for hiding this comment

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

🎉 Thank you!

@srawlins
Copy link
Member

srawlins commented Dec 4, 2022

Approving past the existing linter breakage.

@srawlins srawlins merged commit d558f04 into dart-lang:master Dec 4, 2022
@parlough parlough deleted the fix/error-when-using-base-href branch December 4, 2022 18:22
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.

"Enter" to see full search results doesn't work
2 participants