[webkit-dev] Adding Persistent Position

Darin Adler darin at apple.com
Thu Jun 23 15:08:14 PDT 2011


On Jun 23, 2011, at 2:43 PM, Ryosuke Niwa wrote:

> I think we need a PersistentPosition that auto-corrects itself and survives DOM mutations without being orphaned; similar to how Range is automatically updated.

I’m not sure that auto-correction has a simple unambiguous meaning. There are rules for what happens to Range objects when a document is modified, but I’m not sure they meet anyones expectations for “still representing the same position”.

> Why?
> 	• ReplaceSelectionCommand and DeleteSelectionCommand have wrapper for removeNode, deleteTextFromNode, etc... to update Positions they maintain.
> 	• InsertListCommand and IndentOutdentCommand crash all the time because Position get lost in moveParagraph, etc...
> 	• FrameSelection has a special function that gets called whenever Node is removed to update its end points but VisibleSelection doesn't so endingSelection() becomes stale very easily compared to FrameSelection.

My main concern about this new class would be that it sounds a lot like Range and a lot like Position yet seems likely it won’t share much code with either.

I think that “Persistent” is not the right name for this. A normal Position works just fine. It sticks to the node it’s pointing to. The difference in the new class is that it sticks to something other than the node. What does it stick to? The document?

> On the other hand, adding this new class and instantiating them as member variables of subclasses of CompositeEditCommand may impact performance because CompositeEditCommand stays on the memory even after it's pushed into undo stack.

Doesn’t seem likely to be a practical problem, but I could be wrong.

> Also, maintaining a pointer (probably from Document) to stack object (although member variables of CompositeEditCommand will end up living in heap) seems like a dangerous design

I don’t think that’s dangerous. It’s fine to have pointers into objects, whether on stack or heap, as long as they are maintained correctly, and disconnected when the objects are destroyed.

> To address the performance impact, we can modify CompositeEditCommand so that it won't be added to the undo stack. Instead, CompositeEditCommand will create an EditCommandComposition that stores a tree of SimpleEditCommands that inherits from EditCommand and will be added to the undo stack.

This is a good change.

As I see it, a CompositeEditCommand is the “smarts” to do a command the first time, and should be deallocated as soon as the command has been executed. What’s saved behind for undo should be just a string of undoable editing steps, along with things like the command name for the undo menu item.

Long term I don’t think we need a tree of undoable editing steps. I suspect a vector of them will do fine.

But as an incremental step, I think it would be great to get the smarts for doing commands out of the tree completely. Your specific proposal sounds like a step in the right direction. Later the CompositeEditCommand classes would stop inheriting from EditCommand at all.

    -- Darin



More information about the webkit-dev mailing list