Summary: | Add the ability to search the AccessibilityObject cache | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Samuel White <samuel_white> | ||||||||||||||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Enhancement | CC: | cfleizach, dglazkov, gustavo.noronha, gustavo, samuel_white, samuel.white, webkit.review.bot, xan.lopez | ||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | PC | ||||||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||||||
Attachments: |
|
Description
Samuel White
2011-07-21 16:55:07 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.
apparently only a commit can submit a patch for review... or you have to press that button that says submit 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 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 Created attachment 102288 [details]
Fixed original patch.
Previous patch with fix.
Comment on attachment 102288 [details]
Fixed original patch.
looks good. r=me
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 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 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 on attachment 102288 [details]
Fixed original patch.
we forgot the changeLog... please run ./Tools/Scripts/prepare-Changelog
Created attachment 102317 [details]
Added required change log info
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
Created attachment 102345 [details]
Updated patch.
Added additional ChangeLog comments.
r=me Comment on attachment 102345 [details] Updated patch. Clearing flags on attachment: 102345 Committed r92000: <http://trac.webkit.org/changeset/92000> 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 on attachment 102619 [details] Follow-up patch. Attachment 102619 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9215097 (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 Created attachment 102937 [details]
Fixed patch.
Stub fixes as suggested.
Comment on attachment 102937 [details] Fixed patch. Attachment 102937 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9306525 Created attachment 103162 [details]
Additional patch fixes.
Fixed stub error found in previous patch.
Comment on attachment 103162 [details]
Additional patch fixes.
Ready for review.
Comment on attachment 103162 [details]
Additional patch fixes.
thanks sam. I think this looks good. r=me
Comment on attachment 103162 [details] Additional patch fixes. Clearing flags on attachment: 103162 Committed r92571: <http://trac.webkit.org/changeset/92571> 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 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?
(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. (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. 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 (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 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). |