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 ...
Created attachment 50515 [details] patch 0.1 Calls element->scrollIntoView() before element->focus();
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.
Created attachment 50711 [details] patch 0.3 - same as 0.2 + adds a better ChangeLog entry Simon, could you help me w/ that one?
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
> > +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)
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.