Skip to content
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

URL href mismatch from browsers #36887

Closed
addaleax opened this issue Jan 12, 2021 · 2 comments
Closed

URL href mismatch from browsers #36887

addaleax opened this issue Jan 12, 2021 · 2 comments
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@addaleax
Copy link
Member

  • Version: master
  • Platform: any
  • Subsystem: url

What is the expected behavior?

> new (class extends URL { get hostname() { return 'twitter.com'; } })('http://google.com').href
'http://google.com/'

(I mean, I’m not 100 % sure, but this is what Firefox and Chromium return, and it’s what I would expect as well)

What do you see instead?

> new (class extends URL { get hostname() { return 'twitter.com'; } })('http://google.com').href
'http://twitter.com/'

@nodejs/url

@addaleax addaleax added the whatwg-url Issues and PRs related to the WHATWG URL implementation. label Jan 12, 2021
@domenic
Copy link
Contributor

domenic commented Jan 12, 2021

Good find. Indeed, monkeypatching public APIs like hostname (whether via Object.defineProperty or via inheritance) should not impact the internal algorithms that back other public APIs like href.

@jasnell
Copy link
Member

jasnell commented Jan 12, 2021

Yeah, I agree the behavior should align with browsers.

Lxxyx added a commit to Lxxyx/node that referenced this issue Jan 14, 2021
@aduh95 aduh95 closed this as completed in 9566083 Jan 18, 2021
ruyadorno pushed a commit that referenced this issue Jan 22, 2021
Fixes: #36887

PR-URL: #36903
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #36887

PR-URL: #36903
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants