[webkit-reviews] review denied: [Bug 73821] keyboard event doesn't fire while moving mouse with button pressed : [Attachment 118007] Proposed patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Rakesh
<rakesh.kn at motorola.com>'s request for review:
Bug 73821: keyboard event doesn't fire while moving mouse with button pressed
https://bugs.webkit.org/show_bug.cgi?id=73821

Attachment 118007: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=118007&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118007&action=review


> Source/WebCore/page/EventHandler.cpp:2526
> -	   if (page->mainFrame()->eventHandler()->m_panScrollInProgress ||
m_autoscrollInProgress) {
> +	   if (page->mainFrame()->eventHandler()->m_panScrollInProgress) {
>	       // If a key is pressed while the autoscroll/panScroll is in
progress then we want to stop

Please fix the comment. It is no longer correct after the change.

When was this code introduced, and why?

> LayoutTests/fast/events/autoscroll-should-not-stop-on-keypress.html:11
> +	   var y = document.body.scrollHeight + 100;

Please give "y" a meaningful name.

> LayoutTests/fast/events/autoscroll-should-not-stop-on-keypress.html:19
> +	   debug("Test manually that scrolling deos not stop if we press a key
while autoscroll is happening.");

Typo: does.

> LayoutTests/fast/events/autoscroll-should-not-stop-on-keypress.html:24
> +function testAutoScroll() {
> +    shouldBe('document.body.scrollTop + window.innerHeight',
'document.body.scrollHeight');

What guarantees that scrolling will be done by the time this function is
called?