After the Range.detach() method is called, calls to any further methods should throw a DOMException. The attached test case shows cases where no exception is thrown, where the wrong type of exception is thrown, and where mozilla nightly 2000120521 crashes.

Reassigning to the owner of the Range code.

setting milestone, and priority. anthonyd

fix in hand, will attach for r= and sr= from kin and jst. anthonyd

jst, i need an r= from you. anthonyd

Patch looks good to me, r=jst

fix checked in. anthonyd

This fix has caused textboxes to stop working throughout the application on BeOS and linux. Reopening and will be backing out unless a trivial fix is found soon. (Anyone else up at this hour?)

The textbox problem showed up on my win32 build as well. The changes have been backed out.

Ok, I have a new patch up, that basically does not check for the range being detached in setStart() and setEnd(). It doesnt make sense to test for that anyway, other wise, how would you be able to reset a range? And if you cant reset a range, what would be the point in detach() in the first place? anthonyd also I need r= and sr= again

r=blake

Hmmm... this doesn't seem right. The point of Range.detach() in the DOM spec seems to be to tell the implementation that one is done with a range. (I'm not sure why it would have that rather than allow the implementation's memory management to deal with disposing of the range in the normal way.) Why are ranges being reused after a call to detach()? Perhaps, for our own internal use, we need something that's like detach() but doesn't have the same effect of causing the exceptions?

There's only one place in the entire codebase we call Detach on an nsRange, and that's in editor/base/nsHTMLEditRules.cpp, line 254 and 255 (in nsHTMLEditRules::BeforeEdit). So why don't we just fix the caller and then check in the original patch?

It seems to me, though, that there's a bigger problem here. mIsPositioned is initialized to false, and the only way to make it true is by calling DoSetRange, which is called by SetStart and SetEnd. This means you're not only throwing the exceptions after detach has been called - you're also throwing them before the range has been initialized. Or did I miss something?

I just posted a (somewhat) related question to www-dom: http://lists.w3.org/Archives/Public/www-dom/2001JanMar/0013.html

ok, so it looks like detach is just supposed to be used as a release method. which also means that it is a useless method. the original patch i wrote also broke input fields (text areas) because it tries to call setStart and setEnd on a un positioned range which i was using to determine if the range is detached. this will need to be worked on some more. anthonyd

ok, I've got a new patch for this, which meets the dom spec, and does NOT break input fields. need sr= and r= from everyone again. anthonyd

You might also want to fix the code in nsHTMLEditRules that I mentioned above.

I didn't catch this when I reviewed the earlier patch but I think the changes to nsRange::ToString() are incorrect, calling ToString() on a detached range should IMO not throw an exception, it should simply return an empty string, comments?

the spec says throw an exception, so i made it throw an exception. from the spec: toString Returns the contents of a Range as a string. This string contains only the data characters, not any markup. Return Value DOMString The contents of the Range. Exceptions DOMException INVALID_STATE_ERR: Raised if detach() has already been invoked on this object. anthonyd

Oh, right, I somehow thought ToString was something we had added to Range to make it work nicer in JS, but not! So given that, r=jst.

ok, i got a sr= from kin (given over irc) and a r= from jst. The only thing left before this goes into is jsut to clear up a problem in the editrules code with joe francis. anthonyd

ok, could you take a look at my proposed patch for the rules code change? It is in addition to the dom patch for range (the latest one i added). I know I dont check in everything in the editrules code patch i just added, im just too tired right now to trim down the patch file. anthonyd

a=jfrancis

ok, im gonna check this in. anthonyd

fix checked in anthonyd

catching up on verifications