Bug 23163 – suppress scrolling in Element::updateFocusAppearance

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Cary Clark]

A device with a directional pad may set the focus while navigating, to allow javascript to modify the DOM as appropriate. The same directional pad may navigate by scrolling -- and there's no way to know if the user intended to move the focus or scroll. 

On the desktop, changing the focus also attempts to scroll the focused element into view. This is undesirable for devices that use a directional pad for navigation.

(patch depends on 23145 for the definition of ENABLE_DIRECTIONAL_PAD_NAVIGATION)

[Cary Clark]

Created attachment 26491 [details]
don't scroll when setting focus for devices with directional pad navigation

[Darin Adler]

Comment on attachment 26491 [details]
don't scroll when setting focus for devices with directional pad navigation

It seems to me that the real issue here isn't exactly "directional pad navigation" -- it's more "small screen with explicit scrolling", and I suppose the explicit scrolling part is what's really relevant. I could easily imagine someone using WebKit on a web browser for a TV screen, and using a directional pad on a remote control. It's entirely unclear to me whether the presence of the directional pad would also cause the developers to want this behavior in that case.

I'm going to say r=me with this, but I think we're being a bit unclear on what DIRECTIONAL_PAD_NAVIGATION means and that could become a problem down the line.

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/dom/Element.cpp
Committed r40647

[Eric Seidel (no email)]

Patch was wrong, fixed:

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/dom/Element.cpp
Committed r40654

[Antonio Gomes]

it is interesing that this patch landed, and its BLOCKER did not. It introduced:

=======================================
#if !ENABLE(DIRECTIONAL_PAD_NAVIGATION)
    else if (renderer() && !renderer()->isWidget())
        renderer()->enclosingLayer()->scrollRectToVisible(getRect());
#endif
=======================================

but DIRECTIONAL_PAD_NAVIGATION is not even in Platform.h . Grep'ing for it outputs only:

$ grep -nHR DIRECTIONAL_PAD_NAVIGATION .
./WebCore/dom/Element.cpp:1293:#if !ENABLE(DIRECTIONAL_PAD_NAVIGATION)

cc'ing darin who reviwed and eric who landed it.

[Darin Adler]

This is something that was put in for Android. Beyond that I have little insight into what it is or if it should be removed or kept.