Skip to content

fix(platform-browser): Get correct base path when using "." as base href when serving from the file:// protocol.#53547

Closed
wartab wants to merge 1 commit intoangular:mainfrom
wartab:fix-relative-path
Closed

fix(platform-browser): Get correct base path when using "." as base href when serving from the file:// protocol.#53547
wartab wants to merge 1 commit intoangular:mainfrom
wartab:fix-relative-path

Conversation

@wartab
Copy link
Copy Markdown
Contributor

@wartab wartab commented Dec 13, 2023 •
edited
Loading

Using http://a as the base URL returns / instead of the actual base path when using the file:// protocol. Using document.baseURI addresses this.

Fixes #53546

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #53546

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I'm not sure how to write a test for this.

@pullapprove pullapprove bot requested a review from alxhub December 13, 2023 13:13
crisbeto
crisbeto requested changes Dec 13, 2023
View reviewed changes
packages/platform-browser/src/browser/browser_adapter.ts Outdated
Copy link
Copy Markdown
Member

@crisbeto crisbeto Dec 13, 2023

Choose a reason for hiding this comment

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

location.href includes the full URL which I suspect might throw it off. We should use document.baseURI which is guaranteed to be only the root.

Copy link
Copy Markdown
Contributor Author

@wartab wartab Dec 13, 2023

Choose a reason for hiding this comment

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

Good call, I wasn't sure if that could cause trouble.

crisbeto
crisbeto approved these changes Dec 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto
Copy link
Copy Markdown
Member

crisbeto commented Dec 13, 2023

@wartab can you squash the commits so the linter doesn't complain?

@wartab
fix(platform-browser): Get correct base path when using "." as base h…
5858bf2 
…ref when serving from the file:// protocol.

Using http://a as the base URL returns / instead of the actual base path when using the file:// protocol. Using document.baseURI addresses this.

Fixes angular#53546
@wartab
Copy link
Copy Markdown
Contributor Author

wartab commented Dec 13, 2023

@crisbeto Just done it, also changed the commit message to be accurate

@crisbeto crisbeto added the action: global presubmit The PR is in need of a google3 global presubmit label Dec 13, 2023
@crisbeto
Copy link
Copy Markdown
Member

crisbeto commented Dec 13, 2023

Passing TGP

@crisbeto crisbeto removed the request for review from alxhub December 13, 2023 16:23
@crisbeto crisbeto added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Dec 13, 2023
@alxhub
Copy link
Copy Markdown
Member

alxhub commented Dec 13, 2023

This PR was merged into the repository by commit fdb9cb7.

@alxhub alxhub closed this in fdb9cb7 Dec 13, 2023
alxhub pushed a commit that referenced this pull request Dec 13, 2023
@wartab @alxhub
fix(platform-browser): Get correct base path when using "." as base h…
d5c631b 
…ref when serving from the file:// protocol. (#53547)

Using http://a as the base URL returns / instead of the actual base path when using the file:// protocol. Using document.baseURI addresses this.

Fixes #53546

PR Close #53547
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Dec 13, 2023 •
edited
Loading

@wartab thanks for the fix! 👍

It'd be great if we can add some tests (as a separate PR) for the file:// protocol to avoid regressions in the future. Please let us know if you might be interested in contributing a PR.

@nbintertech nbintertech mentioned this pull request Dec 13, 2023
Closed
@JoostK JoostK mentioned this pull request Dec 13, 2023
Closed
@wartab
Copy link
Copy Markdown
Contributor Author

wartab commented Dec 14, 2023

@AndrewKushnir I can try, however that file specifically doesn't seem to be covered for tests at all (I think), so bear with me if I don't get it right right away.

@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Dec 14, 2023

@wartab thanks for looking into that 👍 Let us know if you have any questions, we'll be happy to help.

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Jan 14, 2024

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 14, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@wartab @ChellappanRajan
fix(platform-browser): Get correct base path when using "." as base h…
44c0cc2 
…ref when serving from the file:// protocol. (angular#53547)

Using http://a as the base URL returns / instead of the actual base path when using the file:// protocol. Using document.baseURI addresses this.

Fixes angular#53546

PR Close angular#53547
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
@wartab @rlmestre
fix(platform-browser): Get correct base path when using "." as base h…
04d861f 
…ref when serving from the file:// protocol. (angular#53547)

Using http://a as the base URL returns / instead of the actual base path when using the file:// protocol. Using document.baseURI addresses this.

Fixes angular#53546

PR Close angular#53547
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
@wartab @amilamen
fix(platform-browser): Get correct base path when using "." as base h…
8dcb5c0 
…ref when serving from the file:// protocol. (angular#53547)

Using http://a as the base URL returns / instead of the actual base path when using the file:// protocol. Using document.baseURI addresses this.

Fixes angular#53546

PR Close angular#53547
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@crisbeto crisbeto crisbeto approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Serving Angular from Electron breaks since @angular/platform-browser 17.0.5

4 participants

@wartab @crisbeto @alxhub @AndrewKushnir