Bug 41160 - Spatial Navigation: make elements in inner frames nested more than 1 level deep focusable
Summary: Spatial Navigation: make elements in inner frames nested more than 1 level de...
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Antonio Gomes
URL: http://www.w3schools.com/TAGS/tryit.a...
Keywords:
Depends on: 41157
Blocks: 18662
  Show dependency tree / graph
 
Reported: 2010-06-24 07:54 PDT by Antonio Gomes
Modified: 2011-04-19 05:15 PDT (History)
3 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 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 ...
Comment 1 Antonio Gomes 2010-06-24 08:15:17 PDT 
Created attachment 59656 [details]
patch v1
Comment 2 Antonio Gomes 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?
Comment 3 Simon Fraser (smfr) 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.
Comment 4 Antonio Gomes 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?
Comment 5 Simon Fraser (smfr) 2010-06-29 14:16:08 PDT 
isDeepDescendantOf?
Comment 6 Antonio Gomes 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/
Comment 7 Antonio Gomes 2010-06-29 14:57:18 PDT 
Created attachment 60062 [details]
(r62179 - r=smfr) patch v2.1 - right patch
Comment 8 Simon Fraser (smfr) 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.
Comment 9 Antonio Gomes 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>
Comment 10 Antonio Gomes 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!
Comment 11 Simon Hausmann 2010-07-01 01:16:06 PDT 
Revision r62179 cherry-picked into qtwebkit-2.0 with commit 18375caeaf6b2cf99c45e1b225e23c6028e1dd68