Skip to content

Fix Microsoft/TypeScript#19277 #311

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

Closed
wants to merge 1 commit into from
Closed

Conversation

Kingwl
Copy link

@Kingwl Kingwl commented Oct 20, 2017

  1. remove readonly
  2. type to Location | string

Fix microsoft/TypeScript#19277

@saschanaz
Copy link
Contributor

While the setter allows string the getter always returns Location not string. I think forcing location.href is reasonable enough.

@Kingwl
Copy link
Author

Kingwl commented Oct 21, 2017

@saschanaz

  1. 'get' and 'set' accessor must have the same type
  2. but that is strange

@saschanaz
Copy link
Contributor

'get' and 'set' accessor must have the same type

In TypeScript. Not in raw JavaScript world, sadly.

@mhegazy mhegazy closed this Nov 7, 2017
@mhegazy mhegazy reopened this Nov 7, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 7, 2017

i am not sure this is the right fix here... we may need to make this any instead..

we have favored reading over writing before, favoring writing over reading seems to just change the problem instead of fixing it.. seems like any is the only "fix" for both cases..

mhegazy added a commit that referenced this pull request Mar 8, 2018
@mhegazy mhegazy mentioned this pull request Mar 8, 2018
@mhegazy mhegazy closed this in #387 Mar 8, 2018
@Kingwl Kingwl deleted the fix-location branch March 8, 2018 01:08
@amoshydra
Copy link

Wanted to highlight that this PR is reverted in this commit 9e88bfd

As of today [email protected] it is not possible to assign type string to Location

window.location = '/'

Error:

Type 'string' is not assignable to type 'Location'.ts(2322)

@saschanaz
Copy link
Contributor

Is there any reason to not use window.location.href?

@amoshydra
Copy link

Is there any reason to not use window.location.href?

Apparently yes. There seems to be some differences between setting location.href vs location.

Quoting

I note that in browsers today, setting window.location = "some string" has special behaviour, see here: stackoverflow.com/questions/2383401/… - see the comments regarding same-site, same-origin and XHR behaviour. – Dai Dec 16 '19 at 5:54

Was trying to create an issue about this in this repo, however, I am not able to setup an environment to proof those differences mention in the discussion.

@saschanaz
Copy link
Contributor

Have you seen that behavior yourself? The mentioned web page doesn't say the href setter will throw (only the getter will), and AFAIK setting window.location simply forwards its value to window.location.href. (See PutForwards=href in the IDL.)

@amoshydra

This comment has been minimized.

@amoshydra
Copy link

amoshydra commented Jan 7, 2020

@saschanaz I now understood what you meant by "doesn't say the href setter will throw". I wasn't reading the mentioned web page in detail earlier.

Upon testing, it is possible to redirect an iframe by setting both location and location.href in context where same-origin policy is enforced.

I have tested this behaviour in different browsers.
  • Chrome 79
  • Firefox 68.3
  • IE 11 (Also emulated IE 5, 7, 8, 9, 10)
  • Edge 44.18362,449 (it crashed for both case, seems to be a separate bug with Edge itself)

Test: https://codepen.io/amoshydra/full/YzPeyYb

All browsers are able to redirect an iframe of different origin by setting its location.href

It seems like all those claims about the special behaviour of setting location made are invalid / unfounded.


I agree with you. Right now I see no reason to not use window.location.href

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.

window.location is not supposed to be a read-only property
4 participants