[webkit-reviews] review denied: [Bug 88793] Implement undoscope attribute. : [Attachment 154370] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Sukolsak Sakshuwong
<sukolsak at google.com>'s request for review:
Bug 88793: Implement undoscope attribute.
https://bugs.webkit.org/show_bug.cgi?id=88793

Attachment 154370: Patch
https://bugs.webkit.org/attachment.cgi?id=154370&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=154370&action=review


> Source/WebCore/dom/Element.cpp:2211
> +    if (!undoScope() || (isContentEditable() && !isRootEditableElement()))
> +	   return 0;

Since we're doing this lazily for -webkit-user-modify, we need to disconnect
the undo manager as needed here.
Also, each method of undo manager needs to check whether undo manager had been
disconnected or not
upfront and throw & disconnect itself as needed.

> Source/WebCore/dom/Element.cpp:2241
> +	   if (node->isContentEditable())
> +	       element->disconnectUndoManager();

This is not right. It's possible to have an editable element within a
non-editable element within an editable element.
e.g. <div undoscope><span contenteditable=false><span contenteditable=true
undoscope>hello</span></span></div>
div.contentEditable=true should NOT disconnect the undoManager on the
inner-most span because the inner most element
had always been editable. r- because of this.