[webkit-reviews] review requested: [Bug 7579] TinyMCE: Implement execCommand(insertImage, ...) : [Attachment 6931] patch

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Tue Mar 7 23:00:54 PST 2006

Justin Garcia <justin.garcia at apple.com> has asked  for review:
Bug 7579: TinyMCE: Implement execCommand(insertImage, ...)

Attachment 6931: patch

------- Additional Comments from Justin Garcia <justin.garcia at apple.com>
> I don't think enabledPaste is a good name for the function any more if it's
> being used for other insertion. Are you sure that's the right function to

I changed this and now use enabledAnySelection.  The enabledXXX functions need
work, I filed 7650.

> I think ReplaceSelectionCommand::prune would read better as a for loop.

I disagree, I left the while loop in.

> shouldPruneNode() will return true for a rendered leaf node (because it has a

> no rendered children).  r-

True, but it will never be passed a leaf because 1) prune is only passed
containers and 2) prune only ascends straight upward.

> Also, "prune" is not an appropriate name because a) it does not necessarily
remove any nodes

I don't think prune implies that nodes will necessarily be removed, only that
they will be removed if they are superfluous.

> b) it may remove more than just the passed node.

To make it clear what may be pruned I made the function into
removeNodeAndPruneAncestors.  This also frees us from worrying that we'll prune

More information about the webkit-reviews mailing list