Summary: | HTML5 promotes DL from specific 'definition list' to superset 'description list'; accessibility strings and accessors should be updated to match. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Craig <jcraig> | ||||||||||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, aboxhall, apinheiro, buildbot, cfleizach, dglazkov, dmazzoni, fishd, gustavo, gyuyoung.kim, jamesr, jcraig, jdiggs, jochen, mifenton, mrobinson, ojan.autocc, peter+ews, rakuco, rniwa, rwlbuis, tkent+wkapi, tonikitoo, webkit.review.bot, yong.li.webkit | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 108291 | ||||||||||||||||
Attachments: |
|
Description
James Craig
2013-01-23 01:59:06 PST
Created attachment 184186 [details]
string and accessors mods, and new layout test.
Patch includes string and accessor changes, as well as a layout test that verifies role/subrole/roleDescription on many HTML elements and all of the ARIA 1.0 roles.
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. Comment on attachment 184186 [details] string and accessors mods, and new layout test. Attachment 184186 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16079111 Comment on attachment 184186 [details] string and accessors mods, and new layout test. Attachment 184186 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/16052028 Comment on attachment 184186 [details] string and accessors mods, and new layout test. View in context: https://bugs.webkit.org/attachment.cgi?id=184186&action=review > Source/WebCore/accessibility/AccessibilityObject.cpp:1396 > + { "definition", DescriptionListTermRole }, if ARIA still has a definition role, then maybe we should continue having a DefiniteListRole, and add a new DescriptionListRole > Source/WebKit/gtk/po/it.po:1260 > msgid "_Clear recent searches" it seems unlikely that you need to update this file > LayoutTests/accessibility/heading-level.html:15 > +<!-- Waiting on PFWG-ISSUE-549 resolution for ARIA 1.1: http://lists.w3.org/Archives/Public/wai-xtech/2013Jan/0024.html --> is this part of another patch? > LayoutTests/accessibility/role-subrole-roledescription-expected.txt:6 > +PASS: A is AXLink//link. subrole only really exists on Mac, so this should probably be a mac test only > LayoutTests/accessibility/role-subrole-roledescription-expected.txt:25 > +PASS: H1 is AXHeading//heading. can you use a format like H1 Role: AXHeading AXSubrole: AXRoleDescription: heading or something that shows which is which (In reply to comment #5) > if ARIA still has a definition role, then maybe we should continue having a DefiniteListRole, and add a new DescriptionListRole Done. Added new "DefinitionRole" for the ARIA role, b/c it's not related to lists. > > Source/WebKit/gtk/po/it.po:1260 > > msgid "_Clear recent searches" > > it seems unlikely that you need to update this file It contained one of the accessors and an was an easy update, but okay. Reverted. > > LayoutTests/accessibility/heading-level.html:15 > > +<!-- Waiting on PFWG-ISSUE-549 resolution for ARIA 1.1: http://lists.w3.org/Archives/Public/wai-xtech/2013Jan/0024.html --> > > is this part of another patch? Removed. > > LayoutTests/accessibility/role-subrole-roledescription-expected.txt:6 > > +PASS: A is AXLink//link. > > subrole only really exists on Mac, so this should probably be a mac test only Done. > > LayoutTests/accessibility/role-subrole-roledescription-expected.txt:25 > > +PASS: H1 is AXHeading//heading. > > can you use a format like > > H1 > Role: AXHeading > AXSubrole: > AXRoleDescription: heading > > or something that shows which is which Done. Created attachment 184376 [details]
Patch take #2
(In reply to comment #2) > Please wait for approval from ..., as this patch contains changes to the Chromium public API. No longer. Clone if needed for public API change. Comment on attachment 184376 [details] Patch take #2 Attachment 184376 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16079486 Comment on attachment 184376 [details] Patch take #2 Attachment 184376 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16073612 New failing tests: platform/mac/accessibility/definition-list-term.html accessibility/lists.html Comment on attachment 184376 [details] Patch take #2 Attachment 184376 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/16098001 Created attachment 184663 [details]
Patch #2
Comment on attachment 184663 [details] Patch #2 Attachment 184663 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/16097680 Comment on attachment 184663 [details] Patch #2 Attachment 184663 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16113347 Created attachment 184831 [details]
Patch #4
Attempting to resolve the chromium test run errors.
Comment on attachment 184831 [details] Patch #4 View in context: https://bugs.webkit.org/attachment.cgi?id=184831&action=review otherwise change looks ok to me. dominic, do you have any comments from chromium side? > LayoutTests/platform/mac/accessibility/role-subrole-roledescription.html:1 > +<!DOCTYPE HTML> this layout test should be in a separate patch. it doesn't really relate to this specific change (In reply to comment #16) > > LayoutTests/platform/mac/accessibility/role-subrole-roledescription.html:1 > > this layout test should be in a separate patch. it doesn't really relate to this specific change I specifically added the test case for the DL/DD/DT elements and the 'definition' role it contains. Adding the rest of the roles and elements was a trivial amount of effort for a worthwhile gain. Comment on attachment 184831 [details] Patch #4 Attachment 184831 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16120892 New failing tests: platform/mac/accessibility/role-subrole-roledescription.html Created attachment 184889 [details]
Patch #5
Same as previous, but without new layout test, which I'll add as a separate patch.
Created attachment 184890 [details]
Patch #5
Previous upload choked.
Chromium side looks good, thanks. No Chromium API changes, just making it compile, so it's fine to commit. Comment on attachment 184890 [details] Patch #5 Clearing flags on attachment: 184890 Committed r140974: <http://trac.webkit.org/changeset/140974> All reviewed patches have been landed. Closing bug. This patch caused accessibility/lists.html to fail on Lion bots. See the bug 108291. |