Closed Bug 14623 Opened 21 years ago Closed 21 years ago

editor strategy for batching/merging transactions needs some work


(Core :: DOM: Editor, defect, P3)






(Reporter: buster, Assigned: mozeditor)


3) I've been overusing batching.  Batching is evil.  I used to think it
such a clear blue sky - being able to nest batches made a lot of
stuff so much
easier to write.  But batching absolutely, positively
breaks any kind of
merging.  You can't use it inside of any code that
might be doing anything that
 is part of a mergable operation.  Most
things we do in response to typing is
supposed to merge: hence, no
batching in any of our typing execution path.

The right solution here is the placeHolderTxn, which was Steve's
answer to the problem of merging together a bunch of disparate
under the auspices of one operation, while still allowing
that overall
operation to merge with a like one executed later.

  However, it's a pain to
carry around a pointer to a placeHolderTxn
everywhere the rules code goes and
then fill it in the first time we
discover that yes, this operation is going to
 have _some_ transactions
in it and not be empty.  There are a lot of places
that can be the
_first_ place to make actual content changes in response to
Deletion of the bogus node is one.  Splitting of mailcites is one.

Actual text insertion is one.  Creating inline style nodes is one.  I
bet I'm
forgetting some, and I bet there will be more later.  Having all
of these
having to call some "check to see if the placeHolderTxn is null
and if so make
one" code (everyplace they might first make a content
change) is pretty darn

  I would prefer that we create one of these placeHolderTxns at the

start of a mergable operation (EditorKeyPress() would probably be the
place) and then at the end of the operation be able to detect
whether anything
actually got done.  If nothing did, somehow the
transaction should vanish.  I
assume we need an addition to the
transaction manager here: something to say
"hey, we know we told you to
do that, and you did, and now it's on your undo
stack, but we've since
realized that transaction did _nothing_ and we want you
to forget about
it".  Otherwise when the user does undo, there will be an
"empty" undo

  Although I've fixed a lot of undo problems this
weekend without taking
this step, it's going to be broken in places until we
either do this, do
something smarter that I havne't thought of but you
hopefully will, or
write lots of ugly code everywhere to check the status of
any existing
placeHolderTxn.  [An example of broken undo still left is undoing
that has returns in it.  The results are pretty hideous right now.]
Summary: editor's use of GetChildNodes needs auditing → editor strategy for batching/merging transactions needs some work
Target Milestone: M12
accepting bug / m12
Closed: 21 years ago
Resolution: --- → FIXED
this is fixed - undo now works like you would expect.  long live undo
I can't verify this one....jfrancis/buster, can you mark this verified-fixed..
verified, per demo to rickg yesterday ;-)
You need to log in before you can comment on or make changes to this bug.