1788513 - location.href set to unparseable URL should throw TypeError

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect, P3)

Description

[Dominic Farolino [:domfarolino]]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36

Steps to reproduce:

Setting location.href to an unparseable URL should throw a TypError, but instead a NS_ERROR_MALFORMED_URI is thrown, which is not even an instanceof Error.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

I guess we're taking the error from necko and not converting it to TypeError.
https://searchfox.org/mozilla-central/rev/9962f512b52f5faba6b5a769b9e89a2a72fb8dbf/dom/base/LocationBase.cpp#198-203,238

Comment 2

[Adam Vandolder [:avandolder]]

Attachment: Bug 1788513 - Setting location.href to a malformed URI throws a TypeError. r=smaug

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa7c85ece2b3
Setting location.href to a malformed URI throws a TypeError. r=smaug

Comment 4

[Atila Butkovits]

Backed out for causing failures at url/failure.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/fbe04b2e33e91528785b1b1baeaa12542e3998aa

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=YbZQY7K4SBegkxygUwi8lQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fa7c85ece2b3c71b429dedd72839fb231b6ddd03

Failure log: https://treeherder.mozilla.org/logviewer?job_id=389309133&repo=autoland&lineNumber=4397

Comment 5

[Adam Vandolder [:avandolder]]

Ah, I didn't see that there was already a WPT for this case. I'll set those to passing now.

Comment 6

[Dominic Farolino [:domfarolino]]

I apologize, but we recently changed the tests (and spec) to throw a "SyntaxError" DOMException instead of a TypeError, since that better matches everything else on the Location object. For more discussion, see https://chromium-review.googlesource.com/c/chromium/src/+/3876468/7#message-00ebca665db5b0e50511416299c9650564813908.

In that case, it is possible that you'll need to update the implementation again. Sorry!

Comment 7

[BugBot [:suhaib / :marco]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:avandolder, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

up to adam to sort out :)

Comment 9

[Adam Vandolder [:avandolder]]

Looking back into this, it seems the test case that broke the autoland (wpt/url/failure.html) is currently broken and crashing, and so is set to pass no matter what. Should the patch just be relanded then? After I fix the new lint error on it.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Crashing?

Comment 11

[Adam Vandolder [:avandolder]]

Yeah - there's a tracking issue for it: https://bugzilla.mozilla.org/show_bug.cgi?id=1790287. I've been unable to get the test to run locally as well. Either way, it seems a recent WPT update (https://bugzilla.mozilla.org/show_bug.cgi?id=1791857) changed the meta file for the test and it should pass now with my previous fix, assuming it doesn't crash.

Comment 12

[Adam Vandolder [:avandolder]]

This issue is no longer relevant, as the specified behaviour was changed from throwing a TypeError to a SyntaxError and this was implemented in bug 1817591.