926890 - Throw JavaScript exceptions for URL

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

DOM exceptions are unstable and using TypeError exclusively here is fine and will allow us to more easily move URL into JavaScript if we decide we want that at some point in the far far future.

Comment 1

[Anne (:annevk)]

As part of this bug we should also consider start throwing for URL.href (not <a>.href and such) as per https://www.w3.org/Bugs/Public/show_bug.cgi?id=23425 I can file a separate bug if desired, but both are fairly minor.

Comment 2

[Andrea Marchesini [:baku]]

Attachment: exceptions.patch

Comment 3

[(no longer active)]

Comment on attachment 817252 [details] [diff] [review]
exceptions.patch

Review of attachment 817252 [details] [diff] [review]:
-----------------------------------------------------------------

Can you please add a similar test for the worker APIs to ensure our sanity?  r=me with that.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: exceptions.patch

testing it on try.

Comment 5

[Masatoshi Kimura [:emk]]

Comment on attachment 817648 [details] [diff] [review]
exceptions.patch

>+++ b/dom/base/URL.cpp
>@@ -186,17 +186,33 @@ URL::GetHref(nsString& aHref) const
>+  nsCOMPtr<nsIURI> uri;
>+  rv = ioService->NewURI(href, nullptr, nullptr, getter_AddRefs(uri));
>+  if (NS_FAILED(rv)) {
>+    aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);

Why it throws DOM SyntaxError instead of ES TypeError?

Comment 6

[Andrea Marchesini [:baku]]

https://tbpl.mozilla.org/?tree=Try&rev=2219e15805e7

I'm going to change NS_ERROR_DOM_SYNTAX_ERR to NS_ERROR_DOM_TYPE_ERR before landing.

Comment 7

[Andrea Marchesini [:baku]]

Attachment: exceptions.patch

Comment 8

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e27f276e8c06

Comment 9

[Masatoshi Kimura [:emk]]

(In reply to Andrea Marchesini (:baku) from comment #6)
> https://tbpl.mozilla.org/?tree=Try&rev=2219e15805e7
> 
> I'm going to change NS_ERROR_DOM_SYNTAX_ERR to  before
> landing.

NS_ERROR_DOM_TYPE_ERR is not a bona fide ES TypeError. It is a DOMException for XPConnect bindings to simulate TypeError. You should use .ThrowTypeError method.
Note that you will have to manually ensure cleanup is performed to use .ThrowTypeError.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e27f276e8c06

Comment 11

[Masatoshi Kimura [:emk]]

Hey, NS_ERROR_DOM_TYPE_ERR (I confused it with NS_ERROR_TYPE_ERR) is one of XPathException. The landed code is completely bogus.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Why wasn't this change reviewed by a DOM peer?

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #12)
> Why wasn't this change reviewed by a DOM peer?

Ehsan? Could you explain?

Comment 14

[(no longer active)]

My mistake, sorry.