Bug 64994 – Add the ability to search the AccessibilityObject cache

• Component: Accessibility
• Status: RESOLVED FIXED

[Samuel White]

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.

[Samuel White]

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.

[chris fleizach]

apparently only a commit can submit a patch for review... or you have to press that button that says submit

[Gyuyoung Kim]

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

[Early Warning System Bot]

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

[Samuel White]

Created attachment 102288 [details]
Fixed original patch.

Previous patch with fix.

[chris fleizach]

Comment on attachment 102288 [details]
Fixed original patch.

looks good. r=me

[Collabora GTK+ EWS bot]

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

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[chris fleizach]

Comment on attachment 102288 [details]
Fixed original patch.

we forgot the changeLog... please run ./Tools/Scripts/prepare-Changelog

[Samuel White]

Created attachment 102317 [details]
Added required change log info

[chris fleizach]

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

[Samuel White]

Created attachment 102345 [details]
Updated patch.

Added additional ChangeLog comments.

[chris fleizach]

r=me

[WebKit Review Bot]

Comment on attachment 102345 [details]
Updated patch.

Clearing flags on attachment: 102345

Committed r92000: <http://trac.webkit.org/changeset/92000>

[Samuel White]

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.

[Collabora GTK+ EWS bot]

Comment on attachment 102619 [details]
Follow-up patch.

Attachment 102619 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9215097

[chris fleizach]

(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

[Samuel White]

Created attachment 102937 [details]
Fixed patch.

Stub fixes as suggested.

[Gustavo Noronha (kov)]

Comment on attachment 102937 [details]
Fixed patch.

Attachment 102937 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9306525

[Samuel White]

Created attachment 103162 [details]
Additional patch fixes.

Fixed stub error found in previous patch.

[Samuel White]

Comment on attachment 103162 [details]
Additional patch fixes.

Ready for review.

[chris fleizach]

Comment on attachment 103162 [details]
Additional patch fixes.

thanks sam. I think this looks good. r=me

[WebKit Review Bot]

Comment on attachment 103162 [details]
Additional patch fixes.

Clearing flags on attachment: 103162

Committed r92571: <http://trac.webkit.org/changeset/92571>

[Samuel White]

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.

[chris fleizach]

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?

[Samuel White]

(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.

[chris fleizach]

(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.

[Samuel White]

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

[chris fleizach]

(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.

[Eric Seidel (no email)]

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).