132302 - Need to return the right exception for DOM Range exceptions

Status: RESOLVED FIXED

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

Description

[Peter Van der Beken [:peterv]]

Currently we only return DOMException and the error codes are wrong (101 and
102).  We need to implement RangeException with the right errorcodes. Patch
coming up later today.

Comment 1

[Peter Van der Beken [:peterv]]

I think we should take this for mozilla1.0. Returning the wrong exception and
the wrong code is bad and if we don't change it now, we'll be locked in for a
long time. Also, my patch allows us to add new exception classes easily, so if
somebody ever implements some other DOM module that has its own excpetion class
(there are some already), they can do the right thing immediately.

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: v1

This patch implements a nsBaseDOMException with an nsIBaseDOMException
interface. This object also implements the nsIException interface so that the
actual DOM exception classes can all reuse the same code for that. The internal
DOM exception classes (the ones implemented inside the DOM module) inherit from
the base exception class, the external ones keep a base exception as a member
variable and forward the calls. I've also provided some macros to implement an
internal exception or an external exception. I've converted the two existing
exception (DOMException and XPathException) to this new scheme and I've added
RangeException.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 75298 [details] [diff] [review]
v1

sr=jst

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: v1 (XPathException)

Forgot to include the XPathException part (sorry jst). I changed the license in
the new files to triple MPL (no new patch for that).

Comment 5

[Axel Hecht]

Comment on attachment 75298 [details] [diff] [review]
v1

>Index: mozilla/dom/public/nsIBaseDOMException.h
<...>
>+#define IMPL_DOM_EXCEPTION_TAIL(classname, ifname, domname, module,          \
>+                                mapping_function)                            \
<..>
>+NS_DEFINE_CID(kBaseDOMException_CID, NS_BASE_DOM_EXCEPTION_CID);             \
what happens if you have two exception implementations in one file?

>Index: mozilla/dom/public/idl/range/MANIFEST_IDL
/me shakes the patch heavily, but neither the Makefile.in nor the makefile.win
pop up.

>Index: mozilla/dom/public/idl/range/nsIDOMRangeException.idl
<...>
>+#include "domstubs.idl"
naah, too much copy'n'paste

So this should do, but as the thing doesn't build I don't hit a checkbox.
I'd like a word on the kBaseDOMException_CID, too.
Either add a comment warning to use these macros only once per source or
just name that k##domname##BaseDOMException_CID or so.
I wonder if I need to proof read the licenses ;-)

Apart from this, I liked the code better the more I looked.

Comment 6

[Peter Van der Beken [:peterv]]

New patch coming up after I've verified it builds on Windows and Linux.

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v1.1

New patch, the nsXPathException changes still need sr too. Builds on Linux and
Windows (Windows not completely done but seems ok).

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 77453 [details] [diff] [review]
v1.1

sr=jst

Comment 9

[Peter Van der Beken [:peterv]]

Attachment: v1.2

Fixed some of Pike's nits.

Comment 10

[Axel Hecht]

Comment on attachment 77662 [details] [diff] [review]
v1.2

r=me, built and tested on
solaris, both debug and optim.
Copying sr by jst

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 77662 [details] [diff] [review]
v1.2

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 12

[Peter Van der Beken [:peterv]]

Seems Netscape is not interested in taking this fix at this point. If anyone
wants to check this in, feel free.

Comment 13

[Peter Van der Beken [:peterv]]

This was checked in on the trunk by Jonas Sicking

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

taking

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

checked in

Comment 16

[Heikki Toivonen (remove -bugzilla when emailing directly)]

So seems like Jonas checked this in on the 1.0.0 branch (04/21/2002 11:01),
adding keyword.