[webkit-reviews] review denied: [Bug 30655] Only plain text should be copied to clipboard for TextArea. : [Attachment 41705] Proposed Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Oct 25 15:37:43 PDT 2009


Darin Adler <darin at apple.com> has denied Jian Li <jianli at chromium.org>'s
request for review:
Bug 30655: Only plain text should be copied to clipboard for TextArea.
https://bugs.webkit.org/show_bug.cgi?id=30655

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

------- Additional Comments from Darin Adler <darin at apple.com>
It's wrong to change copy, but not cut. This same situation can arise with cut.
This should be in shared code used by both cut and copy.

This breaks the Mac platform because it removes the call to
didWriteSelectionToPasteboard for this case. Other platforms treat that as a
no-op. (For some reason they all call notImplemented, which seems unnecessary).


> +    if (target && target->isCharacterDataNode()) {

I don't think it's helpful to check isCharacterDataNode here. None of the code
inside the if statement depends on the type of the node. And it's almost never
helpful to check isCharacterDataNode instead of isTextNode. The nodes in this
case are going to be text nodes. But there's no need to depend on this
implementation detail. Later we may decide we want to use some type of element
inside textarea for some purpose. I suggest you just remove that check
entirely; the rest of the code should work fine.

review- because of the first two problems above.


More information about the webkit-reviews mailing list