[Webkit-unassigned] [Bug 93643] Preserve styling elements in DeleteSelectionCommand

Wed Aug 15 16:12:02 PDT 2012

https://bugs.webkit.org/show_bug.cgi?id=93643

--- Comment #50 from Darin Adler <darin at apple.com>  2012-08-15 16:12:29 PST ---
(In reply to comment #47)
> > I said isEditable() above where I meant isContentEditable().
> 
> isContentEditable will call rendererIsEditable, however <head> does not have a renderer. Is it okay to keep the string compare?

Good point. We can’t use isContentEditable for this. The real fix is probably to do what Ryosuke suggests and keep this inside the root editable node rather than moving it into the head.

If we do want this, we need to use the already existing string comparisons in HTMLElement::collectStyleForAttribute, not write a separate explicit string compare here at the call site. We don't want multiple call sites that know things like whether this is a case sensitive comparison or not. But also, we'd need to implement the rules for inheritance of editability. The head element itself might not have a contenteditable attribute on it, and need not. I think this check of the attribute is what they call a "bad code smell", which indicates we are taking the wrong approach to this.

> >> Source/WebCore/editing/DeleteSelectionCommand.cpp:430
> >> +            appendNode(clone, head);
> > 
> > You should not have to clone the node. Instead you should do a removeNode followed by an appendNode to undoably move the node.
> > 
> > However, doing that also requires changing the way you walk from each node to the next. Before removing a node you need to call traverseNextSibling to advance to a node that won’t be removed.
> 
> I did a clone because undoApply of AppendNodeCommand will simply remove the appended node
> 
> I did not remove because it is inside selection, and the rest of DeleteSelectionCommand will remove it. Is it okay to reuse the existing DeleteSelectionCommand to remove? Maybe I do not understand it, i will consult Enrica.

If we do an undoable remove of the node before the rest of DeleteSelectionCommand runs, then we’ll get exactly what we want. When undoing we’ll first undo the rest of the deletion, then undo the appending to the head, then undo the removal and end up just the way we started.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.