Bug 95248

Summary: AX: Canvas should have a distinct role
Product: WebKit Reporter: Dominic Mazzoni <dmazzoni>
Component: AccessibilityAssignee: Dominic Mazzoni <dmazzoni>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cfleizach, dglazkov, fishd, jamesr, jchaffraix, jdiggs, pdr, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 95349    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Dominic Mazzoni
Reported 2012-08-28 14:43:03 PDT
On many platforms, the role of "image" or "graphic" is not allowed to have children, so we should use a special role for canvas inside WebCore and let each platform decide on the appropriate role to use, taking into account if the canvas has fallback content.
Attachments
Patch (18.74 KB, patch)
2012-08-28 15:02 PDT, Dominic Mazzoni 		no flags
DetailsFormatted DiffDiff
Patch (24.22 KB, patch)
2012-08-28 15:53 PDT, Dominic Mazzoni 		no flags
DetailsFormatted DiffDiff
Patch for landing (24.18 KB, patch)
2012-08-29 00:07 PDT, Dominic Mazzoni 		no flags
DetailsFormatted DiffDiff
Patch for landing (24.19 KB, patch)
2012-08-29 11:25 PDT, Dominic Mazzoni 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Dominic Mazzoni
Comment 1 2012-08-28 15:02:55 PDT
Created attachment 161067 [details] Patch
WebKit Review Bot
Comment 2 2012-08-28 15:05:16 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Adam Barth
Comment 3 2012-08-28 15:50:16 PDT
Comment on attachment 161067 [details] Patch WebKit API change LGTM.
Dominic Mazzoni
Comment 4 2012-08-28 15:53:33 PDT
Created attachment 161076 [details] Patch Update tests. Moved accessibility/canvas.html to platform/mac since it tests for a Mac-specific role.
chris fleizach
Comment 5 2012-08-28 16:43:30 PDT
Comment on attachment 161076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161076&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1994 > + return false; i don't think we want to ignore the canvas element when it has fallback content. I think we just want to make it a "group" role. we should still be able to gather info from it like an aria-label for example
Dominic Mazzoni
Comment 6 2012-08-28 17:09:38 PDT
Comment on attachment 161076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161076&action=review >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1994 >> + return false; > > i don't think we want to ignore the canvas element when it has fallback content. I think we just want to make it a "group" role. we should still be able to gather info from it like an aria-label for example This is in accessibilityIsIgnored, so false means "don't ignore". If it has fallback content, we'll always include it and we'll make it a group role, If it does not have any fallback content, we'll ignore it if its size is 1x1, or if it has no title, description, or help.
chris fleizach
Comment 7 2012-08-28 22:35:17 PDT
Comment on attachment 161076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161076&action=review just one question inline. otherwise looks good > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:318 > + Element* element = toElement(node); it doesn't appear that you need to case to Element in this case
Dominic Mazzoni
Comment 8 2012-08-29 00:07:18 PDT
Created attachment 161151 [details] Patch for landing
WebKit Review Bot
Comment 9 2012-08-29 01:05:02 PDT
Comment on attachment 161151 [details] Patch for landing Clearing flags on attachment: 161151 Committed r126972: <http://trac.webkit.org/changeset/126972>
WebKit Review Bot
Comment 10 2012-08-29 01:05:07 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 11 2012-08-29 08:56:49 PDT
Re-opened since this is blocked by 95349
Julien Chaffraix
Comment 12 2012-08-29 09:03:14 PDT
Link to the test failures: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=accessibility%2Fcanvas-description-and-role.html http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20%28Tests%29/r127000%20%282336%29/results.html The failure is the same in both cases: -PASS axContainer.childrenCount is 2 +FAIL axContainer.childrenCount should be 2. Was 4.
Dominic Mazzoni
Comment 13 2012-08-29 11:25:21 PDT
Created attachment 161273 [details] Patch for landing
WebKit Review Bot
Comment 14 2012-08-29 19:14:21 PDT
Comment on attachment 161273 [details] Patch for landing Clearing flags on attachment: 161273 Committed r127084: <http://trac.webkit.org/changeset/127084>
WebKit Review Bot
Comment 15 2012-08-29 19:14:26 PDT
All reviewed patches have been landed. Closing bug.
Joanmarie Diggs
Comment 16 2012-09-01 06:51:00 PDT
Dominic, since we have a test without results for Gtk, I was looking at the test. Was there a reason you mapped the new CanvasRole to ATK_ROLE_IMAGE in the Gtk platform instead of ATK_ROLE_CANVAS?
Dominic Mazzoni
Comment 17 2012-09-04 11:24:31 PDT
(In reply to comment #16) > Dominic, since we have a test without results for Gtk, I was looking at the test. Was there a reason you mapped the new CanvasRole to ATK_ROLE_IMAGE in the Gtk platform instead of ATK_ROLE_CANVAS? See comment on https://bugs.webkit.org/show_bug.cgi?id=95644
Note You need to log in before you can comment on or make changes to this bug.