Implement various checks on the Location setters

[gterzian]

Spec: https://html.spec.whatwg.org/multipage/history.html#location

See all attributes that say "Can be set, to navigate to the given URL.".

Code: See all the Set_ methods at

servo/components/script/dom/location.rs

Line 78 in 7a5ce04

impl LocationMethods for Location {

As well as set_url_component which is used by those methods:

servo/components/script/dom/location.rs

Line 45 in 7a5ce04

fn set_url_component(&self, value: USVString, setter: fn(&mut ServoUrl, USVString)) {

Currently failing tests(and maybe others):

• https://github.com/servo/servo/blob/master/tests/wpt/metadata/html/browsers/history/the-location-interface/location-protocol-setter.html.ini
• https://github.com/servo/servo/blob/master/tests/wpt/metadata/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html.ini
• https://github.com/servo/servo/blob/master/tests/wpt/metadata/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html.ini

And the actual tests are found in https://github.com/servo/servo/tree/master/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface

A start was made at https://github.com/servo/servo/pull/23368/files#diff-45df5e6ab4c1d79fae956506be147b62R244

and https://github.com/servo/servo/pull/23368/files#diff-c1196726ff21e3563c663b049214dafeL21

Note the only gotcha is that the ServoUrl contains an Arc, so you can't manipulate it directly when you do the initial parse to then perform the check, as that will actually change the url of the window. So instead you'll need to get a copy, as is for example done at https://github.com/servo/servo/pull/23368/files#diff-45df5e6ab4c1d79fae956506be147b62R244 (or you need to perform the checks without manipulating the current url)

[gterzian]

As a bonus you can also return a security error when the spec says to, see for how that is done:

servo/components/script/dom/location.rs

Line 61 in d180f76

Err(Error::Security)

[braddunbar]

I'll work on this!

[gterzian]

Thanks, let us know how it goes! @braddunbar

[gterzian]

@braddunbar How is this going? let us know if you have any questions, you can also just open the PR with a work in progress and ask questions from there...

[braddunbar]

Good! I’ll send a pull this weekend.

[jdm]

There was a PR that was just missing some specification link comments in #23670, then the author stopped responding.

[UxioAndrade]

@jdm Hi, I would like to work on this. Do you think this would be a nice issue for a first contribution?

[gterzian]

@UxioAndrade Absolutely! (Sorry for answering in lieu of @jdm)

If you look at #23670, you can see that most of the work has already been done by @braddunbar, and there are just a few last comments that need to be addressed.

If it's not clear what those last comments are, let us know.

[UxioAndrade]

@gterzian Great, thank you!

[utsavoza]

Hey @gterzian @UxioAndrade, is it ok for me to take up this issue if we are not making progress on it currently?

[gterzian]

Yes it would be great to finally close this one, there is not much work left I think(see the open PR). Perhaps give @UxioAndrade a couple of days to respond? In any case, there is always more work to do on this project!

[utsavoza]

Sure.

[gterzian]

For posterity, I think the easy less label was a bit of an understatement, since the spec is actually rather complicated.