[webkit-reviews] review granted: [Bug 61734] [GTK] Support smart replace for the pasteboard : [Attachment 95496] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 61734: [GTK] Support smart replace for the pasteboard
https://bugs.webkit.org/show_bug.cgi?id=61734

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

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

toggleSmartInsertDelete seems odd to me but maybe it's gtk-style.

> Source/WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:106
> -    GRefPtr<GtkTargetList>
targetList(PasteboardHelper::defaultPasteboardHelper()->targetListForDataObject
(dataObject.get()));
> +    GRefPtr<GtkTargetList>
targetList(PasteboardHelper::defaultPasteboardHelper()->targetListForDataObject
(dataObject.get(), false));

Can we instead set the default value be false?	This is a classic case of enum
being superior to bool.

> Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:617
> +    if (client->smartInsertDeleteEnabled() != enabled)
> +	   client->toggleSmartInsertDelete();

If we had setSmartInsertDeleteEnabled, then we shouldn't need to have an if
statement here.

> Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:429
> +void EditorClient::toggleSmartInsertDelete()
> +{
> +    m_smartInsertDeleteEnabled = !m_smartInsertDeleteEnabled;
> +}

We normally add setSmartInsertDeleteEnabled(bool).