Bug 107650 – HTML5 promotes DL from specific 'definition list' to superset 'description list'; accessibility strings and accessors should be updated to match.

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[James Craig]

HTML5 promotes DL from specific 'definition list' to superset 'description list' which includes, but is not not limited to, definitions. Accessibility strings and accessors should be updated to match. e.g. accessors (e.g. AXDefinitionListDefinitionText) should reference description lists, and role description strings should change from 'definition' to 'description'

[James Craig]

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.

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[WebKit Review Bot]

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

[Peter Beverloo (cr-android ews)]

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

[chris fleizach]

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

[James Craig]

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

[James Craig]

Created attachment 184376 [details]
Patch take #2

[James Craig]

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

[WebKit Review Bot]

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

[Build Bot]

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

[Peter Beverloo (cr-android ews)]

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

[James Craig]

Created attachment 184663 [details]
Patch #2

[Peter Beverloo (cr-android ews)]

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

[WebKit Review Bot]

Comment on attachment 184663 [details]
Patch #2

Attachment 184663 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16113347

[James Craig]

Created attachment 184831 [details]
Patch #4

Attempting to resolve the chromium test run errors.

[chris fleizach]

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

[James Craig]

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

[Build Bot]

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

[James Craig]

Created attachment 184889 [details]
Patch #5

Same as previous, but without new layout test, which I'll add as a separate patch.

[James Craig]

Created attachment 184890 [details]
Patch #5

Previous upload choked.

[Dominic Mazzoni]

Chromium side looks good, thanks. No Chromium API changes, just making it compile, so it's fine to commit.

[WebKit Review Bot]

Comment on attachment 184890 [details]
Patch #5

Clearing flags on attachment: 184890

Committed r140974: <http://trac.webkit.org/changeset/140974>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

This patch caused accessibility/lists.html to fail on Lion bots. See the bug 108291.