Bug 23163

Summary: suppress scrolling in Element::updateFocusAppearance
Product: WebKit Reporter: Cary Clark <caryclark>
Component: DOMAssignee: Cary Clark <caryclark>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   
Bug Depends on: 23145    
Bug Blocks:    
Attachments:
Description Flags
don't scroll when setting focus for devices with directional pad navigation darin: review+

Description Cary Clark 2009-01-07 07:33:26 PST
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)
Comment 1 Cary Clark 2009-01-07 07:39:25 PST
Created attachment 26491 [details]
don't scroll when setting focus for devices with directional pad navigation
Comment 2 Darin Adler 2009-01-10 15:49:24 PST
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.
Comment 3 Eric Seidel (no email) 2009-02-04 16:36:16 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/dom/Element.cpp
Committed r40647
Comment 4 Eric Seidel (no email) 2009-02-04 17:28:07 PST
Patch was wrong, fixed:

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/dom/Element.cpp
Committed r40654
Comment 5 Antonio Gomes 2010-01-28 07:39:45 PST
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.
Comment 6 Darin Adler 2010-01-29 13:41:39 PST
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.