[webkit-reviews] review denied: [Bug 49143] [GTK] The WebKitWebView should expose a set of signals encapsulating the behavior of the EditorClient : [Attachment 73198] Add editing dumps

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 49143: [GTK] The WebKitWebView should expose a set of signals encapsulating
the behavior of the EditorClient
https://bugs.webkit.org/show_bug.cgi?id=49143

Attachment 73198: Add editing dumps
https://bugs.webkit.org/attachment.cgi?id=73198&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=73198&action=review

Holy huge patch Batman.

> WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:367
>  {

Since you are not actually implementing this (why?) you should not add the name
to the parameter.

> WebKit/gtk/webkit/webkitwebview.cpp:153
>      CONSOLE_MESSAGE,

This is an ABI break I believe, don't move the signal enum aronud.

> WebKit/gtk/webkit/webkitwebview.cpp:1392
> +

Is this something like "the first handler wins" or?

> WebKit/gtk/webkit/webkitwebview.cpp:2700
>  

Hrm, is it really useful for subclasses to be able to override this?

> WebKit/gtk/webkit/webkitwebview.h:154
>  

This would be an ABI break; you have to remove one padding.