While working on bug 143338 I found a few bugs and limitations in nsRangeUpdater. 1) nsRangeUpdater does not protect against multiple registrations of the same nsRangeItem, which can lead to multiple adjustment of te same range item. 2) nsRangeUpdater assumed that it had ownership of all nsRangeItems passed to it, and that they were all on the heap (it called delete to free them). This assumption ended up not being very convenient for the caller. 3) nsRangeUpdater notification methods were incomplete for deletion. While DeleteText() and DeleteNode() called into nsRangeUpdater, DeleteSelection() didn't. 4) nsRangeUpdater had some dead code. Next is a patch for all this, which also fixes some broken comments, unused variables, etc, here and there.

The only suxy part of this patch is that I had to move the deletion notifyers for nsRangeUpdater down into the delete transactions themselves, further poluting their api. But since most of the txns already have a poluted api (taking an nsIEditor as an Init param, usually) this is just more sauce for the goose.

Comment on attachment 105067 [details] [diff] [review] patch to editor/libeditor/base in DeleteElementTxn.cpp, around line 113, fix this typo: nlike (unlike?) r=brade

Comment on attachment 105067 [details] [diff] [review] patch to editor/libeditor/base ==== Just in case ... should we be initializing mRangeUpdater in the constructors for DeleteElementTxn and DeleteRangeTxn like we do in DeleteTextTxn? ==== Do we have to be concerned about the fact that DeleteTextTxn will call |mRangeUpdater->SelAdjDeleteText()| during a redo? I was just curious since the range updater is never used during a redo of a DeleteElementTxn.

I fixed brade's typo; I corected the constructors for DeleteElementTxn and DeleteRangeTxn (good catch); and I added range updating to DeleteElementTxn::RedoTransaction() (another good catch). Fix landed on trunk.

oops. can't check this in yet becasue it depends on patch to 143338, which is not yet reviewed. reopening.

fix landed on trunk.

marking...

self verifying this (preventive maintainance only...)