:Ms2ger (he/him; ⌚ UTC+1/+2) Reporter
	Description • 9 years ago

It's no longer in the DOM spec.

However, note:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=24732
https://www.w3.org/Bugs/Public/show_bug.cgi?id=27727

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 1 • 8 years ago

Chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=460725
FileAPI issue: https://github.com/w3c/FileAPI/issues/11

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 2 • 7 years ago

So this is used by DOMRequest.  Do we want DOMRequest to still exist?  Should it be replaced by Promises?

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 5 • 7 years ago

So when I changed IDB to use DOMException instead of DOMError, the wpt test IndexedDB/error-attributes.html fails because now suddenly the ConstraintError returned by IDBObjectStore.add gets caught by window.addEventListener("error",...):

https://treeherder.mozilla.org/logviewer.html#?job_id=121430709&repo=try

Somehow when it was DOMError this didn't happen, but with DOMException it does.  Any idea why, and how to fix it?

	Boris Zbarsky [:bzbarsky]
	Comment 6 • 7 years ago

No idea offhand; I'd have to debug.  At first glance this seems pretty weird: the only way I know of for a DOMException to get caught by an error listener like that is if it actually gets thrown, but I'd expect anything that gets thrown to be thus caught.

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 7 • 7 years ago

The same bug appears in <http://w3c-test.org/html/infrastructure/common-dom-interfaces/collections/domstringlist.html>, the harness fails with an error.  I filed bug 1389913.

	Aryeh Gregor (:ayg) (no longer with Mozilla)
	Comment 8 • 7 years ago

.message and .name getters are called by 1.92% and 2.05% of pages respectively:

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-08-21&keys=__none__!__none__!__none__&max_channel_version=nightly%252F57&measure=USE_COUNTER2_DOMERROR_MESSAGE_getter_PAGE&min_channel_version=null&processType=*&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2017-08-17&table=0&trim=1&use_submission_date=0

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-08-21&keys=__none__!__none__!__none__&max_channel_version=nightly%252F57&measure=USE_COUNTER2_DOMERROR_NAME_getter_PAGE&min_channel_version=null&processType=*&product=Firefox&sanitize=0&sort_keys=submissions&start_date=2017-08-17&table=0&trim=1&use_submission_date=0

That seems far too high to remove, although on the other hand, pages might not break if they hit DOMException's .message and .name instead (they'd just get an unexpected value).

	Andrea Marchesini [:baku] Assignee
	Comment 9 • 7 years ago

I just rebased the existing patch. I also wrote a patch for bug 1389913.
This patch is green on try.

Note that Chromium doesn't have removed the DOMError constructor yet.
But, following https://www.chromestatus.com/metrics/feature/popularity

DOMError CTOR is used by 0.000071%

I suggest to keep the DOMError CTOR but remove it from the webIDL interfaces.

smaug, if you prefer I can split the patch per component or per API.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 10 • 7 years ago

What you mean with "keep the DOMError CTOR but remove it from the webIDL interfaces"?

	Andrea Marchesini [:baku] Assignee
	Comment 11 • 7 years ago

With this patch ports FileReader.error to DOMException. Same for IDB and elsewhere.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 12 • 7 years ago

But the patch removes DOMError interface, meaning also the constructor. So, are you proposing something else than what the patch does?

	Andrea Marchesini [:baku] Assignee
	Comment 13 • 7 years ago

(In reply to Olli Pettay [:smaug] from comment #12)
> But the patch removes DOMError interface, meaning also the constructor. So,
> are you proposing something else than what the patch does?

Yes, I ask a feedback about this approach. Maybe a NI was better, but I also wanted you to take a look at the patch.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 14 • 7 years ago

Comment on attachment 8912222 [details] [diff] [review]
Bug 1120178 - Remove DOMError

So better to keep the interface for now.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 18 • 7 years ago

Comment on attachment 8912539 [details] [diff] [review]
domError.patch

>+++ b/dom/imptests/html/dom/test_interfaces.html
>@@ -5,21 +5,16 @@
> <script src=/resources/testharnessreport.js></script>
> <script src=/resources/WebIDLParser.js></script>
> <script src=/resources/idlharness.js></script>
> 
> <h1>DOM IDL tests</h1>
> <div id=log></div>
> 
> <script type=text/plain>
>-[Constructor(DOMString name)]
>-interface DOMError {
>-  readonly attribute DOMString name;
>-};
>-
Why this?

>+++ b/dom/workers/test/serviceworkers/test_serviceworker_interfaces.js
>@@ -101,18 +101,16 @@ var interfaceNamesInGlobalScope =
>     "Crypto",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "CustomEvent",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "Directory",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "DOMCursor",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>-    "DOMError",
>-// IMPORTANT: Do not change this list without review from a DOM peer!
This doesn't look right.

>+++ b/dom/workers/test/test_worker_interfaces.js
>@@ -99,18 +99,16 @@ var interfaceNamesInGlobalScope =
>     "CustomEvent",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "DedicatedWorkerGlobalScope",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "Directory",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>     "DOMCursor",
> // IMPORTANT: Do not change this list without review from a DOM peer!
>-    "DOMError",
>-// IMPORTANT: Do not change this list without review from a DOM peer!
Nor this


I assume this doesn't pass on try. I'd prefer to look at a patch which does pass all the tests.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 20 • 7 years ago

Comment on attachment 8913105 [details] [diff] [review]
domError.patch

>+++ b/dom/base/test/mochitest.ini
>@@ -653,17 +653,16 @@ skip-if = toolkit == 'android' #bug 9041
> [test_domrequest.html]
> [test_domwindowutils.html]
> [test_e4x_for_each.html]
> [test_element.matches.html]
> [test_element_closest.html]
> [test_elementTraversal.html]
> [test_encodeToStringWithMaxLength.html]
> [test_encodeToStringWithRequiresReinitAfterOutput.html]
>-[test_error.html]
Please don't remove this.

>diff --git a/dom/base/test/test_error.html b/dom/base/test/test_error.html
>deleted file mode 100644
... please don't remove this file

	Pulsebot
	Comment 21 • 7 years ago

Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6ecd2cc9217
Migrate DOMError to DOMExtension in FileReader, IndexedDB, DOMRequest and so on, r=smaug

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 22 • 7 years ago
bugherder

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

	Kohei Yoshino [:kohei]
	Comment 23 • 7 years ago

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/domerror-has-been-removed-in-favour-of-domexception/

	Kohei Yoshino [:kohei]
	Comment 24 • 7 years ago

Forgot to say: This change requires an "intent to unship" post, maybe? I couldn't find one on the dev-platform list.

	Chris Mills [:cmills]
	Comment 25 • 7 years ago

I *think* I've documented this one sufficiently.

I've updated mentions of DOMError to DOMException on appropriate pages, and mentioned when the change occurred in the browser compat notes:

https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/createOffer
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/setRemoteDescription
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/createAnswer
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/setLocalDescription
https://developer.mozilla.org/en-US/docs/Web/API/FileReader/error
https://developer.mozilla.org/en-US/docs/Web/API/IDBRequest/error
https://developer.mozilla.org/en-US/docs/Web/API/IDBTransaction/error

I've marked DOMError as deprecated:

https://developer.mozilla.org/en-US/docs/Web/API/DOMError

I've updated the DOMException pages (although I've just included the stuff listed in the WebIDL spec, not the ExceptionMembers stuff listed in the Gecko source code as I didn't think it sounded very relevant to web developers):

https://developer.mozilla.org/en-US/docs/Web/API/DOMException (and child pages)

I've put a note in the Fx58 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/58#DOM

Let me know if you think this needs anything else!