Bug 39195

Summary: Spatial Navigation: refactor scrollInDirection to work with scrollable content
Product: WebKit Reporter: Antonio Gomes <tonikitoo>
Component: AccessibilityAssignee: Antonio Gomes <tonikitoo>
Status: CLOSED FIXED    
Severity: Normal CC: hausmann, kenneth, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 18662, 36463, 39217    
Bug Blocks:    
Attachments:
Description Flags
patch v0.1
none
patch v1
none
(committed in r61298, reviewed by smfr) patch v2 none

Description Antonio Gomes 2010-05-16 21:06:40 PDT
scrollInDirection method in SpatialNavigation.cpp currently does not works fine for focusable elements in overflowed content.

it needs a refactoring for that.
Comment 1 Antonio Gomes 2010-05-16 21:18:15 PDT
Created attachment 56208 [details]
patch v0.1

Not putting up for review because it needs the blocker bug to land first, and I will work on layout tests for this
Comment 2 Antonio Gomes 2010-05-17 10:37:50 PDT
Created attachment 56249 [details]
patch v1

Not up for review because it relies on patch in bug 39217
Comment 3 Antonio Gomes 2010-06-15 09:37:09 PDT
Created attachment 58787 [details]
(committed in r61298, reviewed by smfr) patch v2

Patch ready for review.

Summary: Patch changes the scrollInDirection method to receives as parameter the node that the spatial navigation
found as the one more appropriated to move focus to. If it is in a scrollable container (e.g. <div> with clipped overflow content), it scrolls recursively starting from the container, not the current focused node.

This is a follow up on bug 39217.
Comment 4 Simon Fraser (smfr) 2010-06-16 17:08:36 PDT
Comment on attachment 58787 [details]
(committed in r61298, reviewed by smfr) patch v2

> +++ b/LayoutTests/fast/events/spatial-navigation/snav-only-clipped-overflow-content.html
> @@ -0,0 +1,77 @@
> +<html>
> +  <!--
> +    This test ensures the content overflow traversal correctness of Spatial Navigation
> +    algorithm: if an element if clipped overflow in a scrollable container (e.g. <div>),
> +    scroll-in-direction should happen in the container box.
> +
> +    * Pre-conditions:
> +    1) DRT support for SNav enable/disable.
> +
> +    * Navigation steps:
> +    1) Loads this page, focus goes to "start" automatically.
> +    2) Try to move focus down to the visible focusable element in
> +       scrollable div.
> +
> +    * Expected results: There should have to a scroll action in the container
> +    (div) as an attempt to make the clipped overflow node visible and accessible
> +    via SNav. -->

Don't use SNav as an abbreviation. Out of context, someone might not know what it means.

r=me
Comment 5 Antonio Gomes 2010-06-16 18:03:07 PDT
Comment on attachment 58787 [details]
(committed in r61298, reviewed by smfr) patch v2

Clearing flags on attachment: 58787

Committed r61298: <http://trac.webkit.org/changeset/61298>
Comment 6 Simon Hausmann 2010-06-17 01:35:42 PDT
Revision r61298 cherry-picked into qtwebkit-2.0 with commit 847dfd81c24656af37ed062a38a47e51a9cfa03f