Bug 37803

Summary:

Spatial Navigation: adapt the logic of {deep}findFocusableNodeInDirection to do traversal starting from Node* not Document*

Product:

WebKit

Reporter:

Antonio Gomes <tonikitoo>

Component:

Accessibility

Assignee:

Antonio Gomes <tonikitoo>

Status:

CLOSED FIXED

Severity:

Normal

CC:

hausmann, kenneth, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Bug Depends on:

Bug Blocks:

35784, 36463

Attachments:

Description	Flags
patch v1	tonikitoo: commit-queue-
patch v2	tonikitoo: commit-queue-
(committer in r58882, reviewed by kenneth) patch v3	none

Antonio Gomes

Reported 2010-04-19 08:43:59 PDT

Instead of having findFocusableNodeInDirection and deepFindFocusableNodeInDirection logic working w/ a Document pointer as incoming parameter from callers, I would like to change them to get a Node pointer. Of course it can node be any node, but "scrollable containers" nodes, e.g. <html> (documentElement()), <iframe>, <frame> or scrollable div's. It is a preparation for supporting scrollable content in Spatial Navigation. See bug 36463. patch coming...

Attachments
patch v1 (9.79 KB, patch) 2010-04-19 09:00 PDT, Antonio Gomes	tonikitoo: commit-queue-	Details Formatted Diff Diff
patch v2 (11.80 KB, patch) 2010-04-20 10:52 PDT, Antonio Gomes	tonikitoo: commit-queue-	Details Formatted Diff Diff
(committer in r58882, reviewed by kenneth) patch v3 (11.48 KB, patch) 2010-04-22 06:49 PDT, Antonio Gomes	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Antonio Gomes

Comment 1 2010-04-19 08:47:16 PDT

s/it can node be any node/it can NOT be any node/

Antonio Gomes

Comment 2 2010-04-19 09:00:16 PDT

Created attachment 53682 [details] patch v1

Antonio Gomes

Comment 3 2010-04-20 10:52:31 PDT

Created attachment 53845 [details] patch v2 - Better ChangeLog entry

Antonio Gomes

Comment 4 2010-04-22 06:49:33 PDT

Created attachment 54057 [details] (committer in r58882, reviewed by kenneth) patch v3 Same as patch v2, but fixed a minor bug in an ASSERT code.

Antonio Gomes

Comment 5 2010-04-22 06:50:15 PDT

To help reviewrs, I can summarize patch v3 (attachment 54057 [details]): * changes in advanceFocusDirectionally and findFocusedNodeInDirection are pretty much straighforward: - at the former, instead passing Document* to the latter, it passes Document::firstChild() - a node - - ... and it traverses the tree down from this node. - Further at findFocusedNodeInDirection method, it changes the use of FOR to WHILE, but logic is exactly the same. * in deepFindFocusableNodeInDirection, code is essentially the same, but under a IF now: // Iframe or Frame. + if (container->hasTagName(frameTag) || container->hasTagName(iframeTag)) { (...)

Antonio Gomes

Comment 6 2010-04-26 20:08:16 PDT

Ping review? see comment #5 for an easier reviewing.

Kenneth Rohde Christiansen

Comment 7 2010-05-06 07:20:17 PDT

Comment on attachment 54057 [details] (committer in r58882, reviewed by kenneth) patch v3 > WebCore/page/FocusController.cpp:393 + void FocusController::findFocusableNodeInDirection(Node* outter, Node* focusedNode, outer WebCore/page/FocusController.cpp:398 + ASSERT(outter); outer WebCore/page/FocusController.cpp:404 + Node* candidate = outter; same thing WebCore/page/FocusController.cpp:443 + // Track if focusedNode is a descendant or not of the current container node being processed. no need to add the "or not"

Antonio Gomes

Comment 8 2010-05-06 09:19:05 PDT

Comment on attachment 54057 [details] (committer in r58882, reviewed by kenneth) patch v3 Clearing flags on attachment: 54057 Committed r58882: <http://trac.webkit.org/changeset/58882>

Simon Hausmann

Comment 9 2010-05-07 00:39:59 PDT

Revision r58882 cherry-picked into qtwebkit-2.0 with commit 712e5d5a8c5ff7d974e19bce03c56bf93ad82314

Note You need to log in before you can comment on or make changes to this bug.