Bug 41160

Summary:

Spatial Navigation: make elements in inner frames nested more than 1 level deep focusable

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

URL:

http://www.w3schools.com/TAGS/tryit.asp?filename=tryhtml_iframe

Bug Depends on:

41157

Bug Blocks:

18662

Attachments:

Description	Flags
patch v1	simon.fraser: review-
patch v2 - s/checkDescendanceRecursively/isDeepDescendantOf/	none
(r62179 - r=smfr) patch v2.1 - right patch	none

Antonio Gomes

Reported 2010-06-24 07:54:31 PDT

Spatial navigation does not work fine traversing nested iframe content as in [1]. [1] http://www.w3schools.com/TAGS/tryit.asp?filename=tryhtml_iframe patch coming ...

Attachments
patch v1 (10.76 KB, patch) 2010-06-24 08:15 PDT, Antonio Gomes	simon.fraser: review-	Details Formatted Diff Diff
patch v2 - s/checkDescendanceRecursively/isDeepDescendantOf/ (10.74 KB, patch) 2010-06-29 14:38 PDT, Antonio Gomes	no flags	Details Formatted Diff Diff
(r62179 - r=smfr) patch v2.1 - right patch (10.71 KB, patch) 2010-06-29 14:57 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-06-24 08:15:17 PDT

Created attachment 59656 [details] patch v1

Antonio Gomes

Comment 2 2010-06-25 11:24:45 PDT

(In reply to comment #1) > Created an attachment (id=59656) [details] > patch v1 @Simon, Kenneth: could any of you help with reviewing this patch?

Simon Fraser (smfr)

Comment 3 2010-06-29 12:28:21 PDT

Comment on attachment 59656 [details] patch v1 > +bool checkDescendenceRecursively(Document* baseDocument, Node* node) Check for what? I think this method name could be improved.

Antonio Gomes

Comment 4 2010-06-29 14:12:06 PDT

Thank you for the review. (In reply to comment #3) > (From update of attachment 59656 [details]) > > +bool checkDescendenceRecursively(Document* baseDocument, Node* node) > > Check for what? I think this method name could be improved. This method works like Node::isDescendantOf, but for nested documents. Basically, it checks if a given node is descendant of a given baseDocument. If it is not, it goes recursively up in the document chain checking if the current document is a descendant of baseDocument. Would you have a better name to suggest based on the description?

Simon Fraser (smfr)

Comment 5 2010-06-29 14:16:08 PDT

isDeepDescendantOf?

Antonio Gomes

Comment 6 2010-06-29 14:38:04 PDT

Created attachment 60056 [details] patch v2 - s/checkDescendanceRecursively/isDeepDescendantOf/ As per Simon suggestion, naming the added method as isDeepDescendantOf/

Antonio Gomes

Comment 7 2010-06-29 14:57:18 PDT

Created attachment 60062 [details] (r62179 - r=smfr) patch v2.1 - right patch

Simon Fraser (smfr)

Comment 8 2010-06-29 14:59:37 PDT

Comment on attachment 60062 [details] (r62179 - r=smfr) patch v2.1 - right patch Since this addition is a function, not a method, the name should be more like isNodeDeepDescendentOfDocument(), or perhaps documentTreeContainsNode(). r=me either way.

Antonio Gomes

Comment 9 2010-06-30 05:46:45 PDT

Comment on attachment 60062 [details] (r62179 - r=smfr) patch v2.1 - right patch Clearing flags on attachment: 60062 Committed r62179: <http://trac.webkit.org/changeset/62179>

Antonio Gomes

Comment 10 2010-06-30 05:47:23 PDT

(In reply to comment #8) > (From update of attachment 60062 [details]) > Since this addition is a function, not a method, the name should be more like isNodeDeepDescendentOfDocument(), or perhaps documentTreeContainsNode(). > > r=me either way. I used the former. Thank you!

Simon Hausmann

Comment 11 2010-07-01 01:16:06 PDT

Revision r62179 cherry-picked into qtwebkit-2.0 with commit 18375caeaf6b2cf99c45e1b225e23c6028e1dd68

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