Spatial Navigation: Add a scrollIntoView call when focusing an element.

• Component: Accessibility
• Status: RESOLVED FIXED

[Antonio Gomes]

In the current Spatial Navigation logic, for an element to get the focus it just needs to be partially visible (in current viewport), not completely.

Even calling |element->focus()| does not necessarily bring element's rect to be completely visible in cases where it is already partially visible.

I think it would be nice to call 'element->scrollIntoView' for that.

patch coming ...

[Antonio Gomes]

Created attachment 50515 [details]
patch 0.1

Calls element->scrollIntoView() before element->focus();

[Antonio Gomes]

Created attachment 50614 [details]
patch 0.2 - add a custom scrollIntoView method

Added a helper scrollIntoView in SpatialNavigation.h. There a inflate operation is performed before scrolling.

[Antonio Gomes]

Created attachment 50711 [details]
patch 0.3 - same as 0.2 + adds a better ChangeLog entry

Simon, could you help me w/ that one?

[Simon Fraser (smfr)]

Comment on attachment 50711 [details]
patch 0.3 - same as 0.2 + adds a better ChangeLog entry


> +void scrollIntoView(Element* element)
> +{
> +    // NOTE: Element's scrollIntoView method could had been used here, but
> +    // it is preferable to inflate |element|'s bounding rect a bit before
> +    // scrolling it for accurate reason.
> +    // Element's scrollIntoView method does not provide this flexibility.
> +    static const int fudgeFactor = 2;

Shouldn't this be related to the platform's focus outline width? That information doesn't seem to be available, though.

r=me

[Antonio Gomes]

> > +void scrollIntoView(Element* element)
> > +{
> > +    // NOTE: Element's scrollIntoView method could had been used here, but
> > +    // it is preferable to inflate |element|'s bounding rect a bit before
> > +    // scrolling it for accurate reason.
> > +    // Element's scrollIntoView method does not provide this flexibility.
> > +    static const int fudgeFactor = 2;
> 
> Shouldn't this be related to the platform's focus outline width? That
> information doesn't seem to be available, though.

I can check that, for sure, Simon.

> r=me

thx again


Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/gtk/Skipped
	M	WebCore/ChangeLog
	M	WebCore/page/FocusController.cpp
	M	WebCore/page/SpatialNavigation.cpp
	M	WebCore/page/SpatialNavigation.h
Committed r56057
	M	WebCore/ChangeLog
	M	WebCore/page/SpatialNavigation.h
	M	WebCore/page/FocusController.cpp
	M	WebCore/page/SpatialNavigation.cpp
	M	LayoutTests/platform/gtk/Skipped
	M	LayoutTests/ChangeLog

r56057 = 4d0624cd978ee7b14b78500555f896a081b40466 (trunk)

[Antonio Gomes]

I had a talk to Kenneth about the desired behaviour of scrolling actions when running Spatial Navigation on a mobo device. It turns out that most of the current browser clients that use it (via a fake mouse point or not) keep the focusable element (and the fake mouse point) on the center of the screen when possible. It will be addressed on its own bug. Current behaviour, matches Opera's destop browser.