Closed Bug 926890 Opened 6 years ago Closed 6 years ago

Throw JavaScript exceptions for URL

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: annevk, Assigned: baku)

References

(Depends on 1 open bug, )

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 2 obsolete files)

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.
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.
Attached patch exceptions.patch (obsolete) — Splinter Review
Attachment #817252 - Flags: review?(ehsan)
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.
Attachment #817252 - Flags: review?(ehsan) → review+
Attached patch exceptions.patch (obsolete) — Splinter Review
testing it on try.
Attachment #817252 - Attachment is obsolete: true
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?
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.
Attached patch exceptions.patchSplinter Review
Attachment #817648 - Attachment is obsolete: true
(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.
https://hg.mozilla.org/mozilla-central/rev/e27f276e8c06
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 927610
Hey, NS_ERROR_DOM_TYPE_ERR (I confused it with NS_ERROR_TYPE_ERR) is one of XPathException. The landed code is completely bogus.
Depends on: 927735
Why wasn't this change reviewed by a DOM peer?
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #12)
> Why wasn't this change reviewed by a DOM peer?

Ehsan? Could you explain?
Flags: needinfo?(ehsan)
My mistake, sorry.
Flags: needinfo?(ehsan)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.