[webkit-reviews] review denied: [Bug 88238] Paste X11 Global Selection : [Attachment 162203] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Allan Sandfeld Jensen
<allan.jensen at nokia.com>'s request for review:
Bug 88238: Paste X11 Global Selection
https://bugs.webkit.org/show_bug.cgi?id=88238

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162203&action=review


Thanks for doing this cleanup.	Seems fine in general, just some questions. r-
for lack of a test.

> Source/WebCore/ChangeLog:13
> +	   This patch moves the implementations of global selection from the 
> +	   separate implementations in Qt WebKit, Chromium and GTK to WebCore,
> +	   by implementing a new EditorCommand for pasting the global
selection.

Do we have any layout tests that cover this?  If so, can you name them in the
ChangeLog?  If not, can you write one?

> Source/WebCore/editing/EditorCommand.cpp:931
> +    } else
> +	   frame->editor()->paste();

If it's only supportedFromMenuOrKeyBinding, when do we hit this else case?

> Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:207
>      setSelectionPrimaryClipboardIfNeeded(frame);
> +#elif PLATFORM(QT)
> +    updateGlobalSelection(frame);

It looks like setSelectionPrimaryClipboardIfNeeded and updateGlobalSelection
both do the same thing (take the selection and write it to the selection
clipboard).  Can they share the same function name?