[webkit-reviews] review denied: [Bug 60879] Mouse button release not recognized with QWebView : [Attachment 116914] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Jani Honkonen
<jani.honkonen at digia.com>'s request for review:
Bug 60879: Mouse button release not recognized with QWebView
https://bugs.webkit.org/show_bug.cgi?id=60879

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=116914&action=review


I'm saying r- because of the missing v8 part. I'm not 100% sure neither if this
is generally the correct change. It's probably going to have to be somebody
else to give an r+ there.

Once thing I would strongly suggest is to _not_ test this behaviour embedded in
the touch test but instead create a dedicated layout test, as this isn't really
specific to the touch event handlers, right?

I admit I'm very surprised that no other existing layout test is covering this
particular aspect of the DOM bindings.

> Source/WebCore/ChangeLog:3
> +	   Mouse button release not recognized with QWebView

I suggest to change the title away from making it sound Qt specific towards
something that indicates that this is about changing WebKit's behavior with
regards to event listeners.

> Source/WebCore/ChangeLog:13
> +	   * bindings/scripts/CodeGeneratorJS.pm:
> +	   (GenerateImplementation): Changed EventListener return value from
jsNull() to jsUndefined().

You're going to have to do a similar change in the V8 generator to fix the
Chromium test failure (Chromium is using V9).