[webkit-reviews] review granted: [Bug 88238] Paste X11 Global Selection : [Attachment 162463] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted 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 162463: Patch
https://bugs.webkit.org/attachment.cgi?id=162463&action=review

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


I think it's fine to land this patch, but let's make a follow up bug for adding
a layout test.

For a layout test, I would do the following:
- Add a method to internals that sets the global selection.
- Write a test that calls internals.setGlobalSelection then calls
testRunner.execCommand.

Using internals to set the selection clipboard avoids the possibility of the
user interacting with the system messing up the test results.

> Source/WebKit/chromium/src/EditorClientImpl.h:116
> +    virtual bool supportsGlobalSelection();

Nit: OVERRIDE

> Source/WebKit/chromium/src/WebViewImpl.cpp:647
> +	       editor->command(AtomicString("PasteGlobalSelection")).execute();


Nit: I think it will automatically convert to AtomicString (the constructor
doesn't have explicit).

> Source/WebKit/qt/WebCoreSupport/EditorClientQt.h:108
> +    virtual bool supportsGlobalSelection();

Nit: OVERRIDE