Bug 64994 - Add the ability to search the AccessibilityObject cache
Summary: Add the ability to search the AccessibilityObject cache
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-21 16:55 PDT by Samuel White
Modified: 2011-09-06 15:38 PDT (History)
8 users (show)

See Also:


Attachments
This patch adds some basic functionality that is needed to support the requested search functionality. (11.53 KB, patch)
2011-07-28 12:01 PDT, Samuel White
no flags Details | Formatted Diff | Diff
Fixed original patch. (11.53 KB, patch)
2011-07-28 12:38 PDT, Samuel White
cfleizach: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Added required change log info (13.81 KB, patch)
2011-07-28 17:40 PDT, Samuel White
cfleizach: review-
Details | Formatted Diff | Diff
Updated patch. (14.51 KB, patch)
2011-07-29 04:36 PDT, Samuel White
no flags Details | Formatted Diff | Diff
Follow-up patch. (55.29 KB, patch)
2011-08-02 00:20 PDT, Samuel White
gustavo.noronha: commit-queue-
Details | Formatted Diff | Diff
Fixed patch. (58.03 KB, patch)
2011-08-04 09:47 PDT, Samuel White
gns: commit-queue-
Details | Formatted Diff | Diff
Additional patch fixes. (58.09 KB, patch)
2011-08-06 13:27 PDT, Samuel White
no flags Details | Formatted Diff | Diff
Consistency patch. (1.65 KB, patch)
2011-08-07 18:38 PDT, Samuel White
cfleizach: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel White 2011-07-21 16:55:07 PDT
WebKit lacks both the ability to search through cached AccessibilityObjects using basic search criteria, as well as the API to expose this functionality. This forces external screen readers that provide search functionality to duplicate the accessibility cache externally by using the existing accessibility API before search can be done.
Comment 1 Samuel White 2011-07-28 12:01:15 PDT
Created attachment 102280 [details]
This patch adds some basic functionality that is needed to support the requested search functionality.

The blockquoteLevel function has been moved into AccessibilityObject to make it available to any platform. Layout tests for the new functionality will be included in the following patch that contains the new search functionality.
Comment 2 chris fleizach 2011-07-28 12:25:15 PDT
apparently only a commit can submit a patch for review... or you have to press that button that says submit
Comment 3 Gyuyoung Kim 2011-07-28 12:29:25 PDT
Comment on attachment 102280 [details]
This patch adds some basic functionality that is needed to support the requested search functionality.

Attachment 102280 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9260387
Comment 4 Early Warning System Bot 2011-07-28 12:31:08 PDT
Comment on attachment 102280 [details]
This patch adds some basic functionality that is needed to support the requested search functionality.

Attachment 102280 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9266258
Comment 5 Samuel White 2011-07-28 12:38:24 PDT
Created attachment 102288 [details]
Fixed original patch.

Previous patch with fix.
Comment 6 chris fleizach 2011-07-28 13:56:48 PDT
Comment on attachment 102288 [details]
Fixed original patch.

looks good. r=me
Comment 7 Collabora GTK+ EWS bot 2011-07-28 14:14:50 PDT
Comment on attachment 102280 [details]
This patch adds some basic functionality that is needed to support the requested search functionality.

Attachment 102280 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9262318
Comment 8 WebKit Review Bot 2011-07-28 15:39:30 PDT
Comment on attachment 102280 [details]
This patch adds some basic functionality that is needed to support the requested search functionality.

Attachment 102280 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9260424
Comment 9 WebKit Review Bot 2011-07-28 17:01:58 PDT
Comment on attachment 102288 [details]
Fixed original patch.

Rejecting attachment 102288 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2

Last 500 characters of output:
a3afccd5e69d55cde842eb43b007d884c7c1f637
r91959 = 2dc424772152f666ca085e7cdf850aedd45b4bb6
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9260453
Comment 10 chris fleizach 2011-07-28 17:03:25 PDT
Comment on attachment 102288 [details]
Fixed original patch.

we forgot the changeLog... please run ./Tools/Scripts/prepare-Changelog
Comment 11 Samuel White 2011-07-28 17:40:20 PDT
Created attachment 102317 [details]
Added required change log info
Comment 12 chris fleizach 2011-07-28 17:41:38 PDT
Comment on attachment 102317 [details]
Added required change log info

please add a comment in your change log as to what this patch is for
Comment 13 Samuel White 2011-07-29 04:36:23 PDT
Created attachment 102345 [details]
Updated patch.

Added additional ChangeLog comments.
Comment 14 chris fleizach 2011-07-29 10:22:09 PDT
r=me
Comment 15 WebKit Review Bot 2011-07-29 10:37:46 PDT
Comment on attachment 102345 [details]
Updated patch.

Clearing flags on attachment: 102345

Committed r92000: <http://trac.webkit.org/changeset/92000>
Comment 16 Samuel White 2011-08-02 00:20:06 PDT
Created attachment 102619 [details]
Follow-up patch.

This patch adds the remaining necessary functionality to support basic AccessibilityObject cache searching as well as API for the mac platform to expose it. Also included are a series of new layout tests that validate both the new functionality added in this patch, as well as the new functionality that was added in the prior patch against this bug.
Comment 17 Collabora GTK+ EWS bot 2011-08-02 10:37:16 PDT
Comment on attachment 102619 [details]
Follow-up patch.

Attachment 102619 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9215097
Comment 18 chris fleizach 2011-08-02 23:30:37 PDT
(In reply to comment #17)
> (From update of attachment 102619 [details])
> Attachment 102619 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/9215097

Likely missing GTK stubs in DRT for the new AccessibilityUIelement methods that were added
Comment 19 Samuel White 2011-08-04 09:47:56 PDT
Created attachment 102937 [details]
Fixed patch.

Stub fixes as suggested.
Comment 20 Gustavo Noronha (kov) 2011-08-04 11:59:54 PDT
Comment on attachment 102937 [details]
Fixed patch.

Attachment 102937 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9306525
Comment 21 Samuel White 2011-08-06 13:27:29 PDT
Created attachment 103162 [details]
Additional patch fixes.

Fixed stub error found in previous patch.
Comment 22 Samuel White 2011-08-06 15:28:03 PDT
Comment on attachment 103162 [details]
Additional patch fixes.

Ready for review.
Comment 23 chris fleizach 2011-08-06 20:49:19 PDT
Comment on attachment 103162 [details]
Additional patch fixes.

thanks sam. I think this looks good. r=me
Comment 24 WebKit Review Bot 2011-08-07 02:48:18 PDT
Comment on attachment 103162 [details]
Additional patch fixes.

Clearing flags on attachment: 103162

Committed r92571: <http://trac.webkit.org/changeset/92571>
Comment 25 Samuel White 2011-08-07 18:38:22 PDT
Created attachment 103194 [details]
Consistency patch.

The level values returned by tableLevel() weren't consistent with the headingLevel() and blockquoteLevel() functions. That is, 0 should only be returned when called on objects that aren't tables.
Comment 26 chris fleizach 2011-08-08 08:31:11 PDT
Comment on attachment 103194 [details]
Consistency patch.

can't you just use 
AccessibilityObject* obj = const_cast<this>;

also is there a layout test that needs to be updated to reflect this change?
Comment 27 Samuel White 2011-08-08 09:04:13 PDT
(In reply to comment #26)
> (From update of attachment 103194 [details])
> can't you just use 
> AccessibilityObject* obj = const_cast<this>;

const casting an accessibilityTable to an accessibilityObject is not allowed.

> 
> also is there a layout test that needs to be updated to reflect this change?

The existing layout test for tables will continue to work. This change just makes the level values reported more consistent with other level values. For example, the search-predicate.html layout test currently sees that two level 0 tables are at the same level. After this change it will see that two level 1 tables are at the same level. Further, level 0 will be correctly reserved for objects which aren't tables at all. This is the behavior displayed when one calls headingLevel() on a non-heading.
Comment 28 chris fleizach 2011-08-08 09:35:10 PDT
(In reply to comment #27)
> (In reply to comment #26)
> > (From update of attachment 103194 [details] [details])
> > can't you just use 
> > AccessibilityObject* obj = const_cast<this>;
> 
> const casting an accessibilityTable to an accessibilityObject is not allowed.
> 

I suppose you could do this, but i don't know if it's any easier.

AccessibilityObject* obj = dynamic_cast<AccessibilityObject*>(const_cast<AccessibilityTable*>(this));

At the least, you should be able to use get() instead of getOrCreate() since you know the object exists

axObjectCache()->get(m_renderer)

> > 
> > also is there a layout test that needs to be updated to reflect this change?
> 
> The existing layout test for tables will continue to work. This change just makes the level values reported more consistent with other level values. For example, the search-predicate.html layout test currently sees that two level 0 tables are at the same level. After this change it will see that two level 1 tables are at the same level. Further, level 0 will be correctly reserved for objects which aren't tables at all. This is the behavior displayed when one calls headingLevel() on a non-heading.

Sounds like we should probably add a new layout test to ensure that we get the right levels when we ask a table for it's table level.
Comment 29 Samuel White 2011-08-12 19:08:25 PDT
Since this has no immediate impact on search I opened a new bug for the level issue. For further details and resolution see:

Bug 66180 - AccessibilityObject levels are inconsistent
Comment 30 chris fleizach 2011-08-12 19:12:15 PDT
(In reply to comment #29)
> Since this has no immediate impact on search I opened a new bug for the level issue. For further details and resolution see:
> 
> Bug 66180 - AccessibilityObject levels are inconsistent

great. so we can close this bug.
Comment 31 Eric Seidel (no email) 2011-09-06 15:38:49 PDT
Comment on attachment 102280 [details]
This patch adds some basic functionality that is needed to support the requested search functionality.

Cleared review? from attachment 102280 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).