[webkit-reviews] review denied: [Bug 36003] Enhance GTK DRT implementation to support platform scroll wheel events. : [Attachment 146016] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied José Dapena Paz
<jdapena at igalia.com>'s request for review:
Bug 36003: Enhance GTK DRT implementation to support platform scroll wheel
events.
https://bugs.webkit.org/show_bug.cgi?id=36003

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=146016&action=review


Looks good, just a few style nits. Can't we unskip some tests now?

> Tools/DumpRenderTree/gtk/EventSender.cpp:483
> +    int horizontal = (int)JSValueToNumber(context, arguments[0], exception);

> +    g_return_val_if_fail((!exception || !*exception),
JSValueMakeUndefined(context));
> +    int vertical = (int)JSValueToNumber(context, arguments[1], exception);
> +    g_return_val_if_fail((!exception || !*exception),
JSValueMakeUndefined(context));

You should use static_cast here instead of C-style casts. Is it possible to
just elminate the cast entirely here?

> Tools/DumpRenderTree/gtk/EventSender.cpp:487
> +    gboolean paged = false;
> +    if (argumentCount == 3) {
> +	   paged = JSValueToBoolean(context, arguments[2]);

bool intead of gboolean would be more appropriate here. Why does paged need to
be defined in outer scope? Seems like this could just be:

g_return_val_if_fail(argumentcount < 3 || !JSValueToBoolean(context,
arguments[2]), JSValueMakeUndefined(context));