Bug 49209 – WebKit2 should exit auto scrolling mode when losing focus

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brian Weinstein]

We do not exit pan scrolling mode when switching tabs in WebKit2.

In WebKit1, this was fixed by http://trac.webkit.org/changeset/50007, but the bug still exists in WebKit2.

Steps to reproduce:
1) Open two tabs, one that can scroll
2) Start panscrolling in the tab that can scroll
3) Switch focus to the other tab.

The pan scrolling icon stays active, even though it shouldn't.

[Brian Weinstein]

Created attachment 73285 [details]
[PATCH] Fix

[Adam Roben (:aroben)]

Comment on attachment 73285 [details]
[PATCH] Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=73285&action=review

Is it possible to make a test for this using TestWebKitAPI? You'd have to add code that can simulate a middle-click, and a way to make the WebView lose focus, and a way to detect whether the autoscroll timer was stopped.

> WebKit2/WebProcess/WebPage/WebPage.cpp:671
> +    FocusController* focusController = m_page->focusController();
> +    if (!isFocused)
> +        focusController->focusedOrMainFrame()->eventHandler()->stopAutoscrollTimer();
> +    focusController->setFocused(isFocused);

Should this code be in FocusController::setFocused instead?

[Brian Weinstein]

(In reply to comment #2)
> (From update of attachment 73285 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=73285&action=review
> 
> Is it possible to make a test for this using TestWebKitAPI? You'd have to add code that can simulate a middle-click, and a way to make the WebView lose focus, and a way to detect whether the autoscroll timer was stopped.

If it was a windows specific test, I could make sure the autoscroll timer was stopped by checking the scroll position before and after the timer was supposed to be killed, but I don't know if it's possible to test the gaining and losing focus of the web view.

> 
> > WebKit2/WebProcess/WebPage/WebPage.cpp:671
> > +    FocusController* focusController = m_page->focusController();
> > +    if (!isFocused)
> > +        focusController->focusedOrMainFrame()->eventHandler()->stopAutoscrollTimer();
> > +    focusController->setFocused(isFocused);
> 
> Should this code be in FocusController::setFocused instead?

No reason it shouldn't be, then it wouldn't be something that would need to be duplicated across WebKit platforms.

[Brian Weinstein]

Created attachment 76725 [details]
[PATCH] Fix in WebCore

[WebKit Review Bot]

http://trac.webkit.org/changeset/74170 might have broken Leopard Intel Debug (Tests)

[Eric Seidel (no email)]

Looks like this was landed but not closed.