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

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

Summary: HTML5 promotes DL from specific 'definition list' to superset 'description li...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Accessibility (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Nobody

URL:
Keywords:

Depends on:
Blocks:	108291
	Show dependency tree / graph

Reported:	2013-01-23 01:59 PST by James Craig
Modified:	2013-01-29 21:31 PST (History)
CC List:	25 users (show)

See Also:

Attachments
string and accessors mods, and new layout test. (42.44 KB, patch) 2013-01-23 02:30 PST, James Craig	webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
Patch take #2 (48.77 KB, patch) 2013-01-23 18:38 PST, James Craig	webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
Patch #2 (54.25 KB, patch) 2013-01-24 22:06 PST, James Craig	peter+ews: commit-queue-	Details \| Formatted Diff \| Diff
Patch #4 (57.84 KB, patch) 2013-01-25 15:56 PST, James Craig	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Patch #5 (27.52 KB, application/octet-stream) 2013-01-26 16:09 PST, James Craig	no flags	Details
Patch #5 (27.52 KB, patch) 2013-01-26 16:12 PST, James Craig	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description James Craig 2013-01-23 01:59:06 PST

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'

Comment 1 James Craig 2013-01-23 02:30:42 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.

Comment 2 WebKit Review Bot 2013-01-23 02:47:59 PST

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 3 WebKit Review Bot 2013-01-23 02:54:15 PST

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 4 Peter Beverloo (cr-android ews) 2013-01-23 03:08:24 PST

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 5 chris fleizach 2013-01-23 08:15:08 PST

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

Comment 6 James Craig 2013-01-23 18:36:25 PST

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

Comment 7 James Craig 2013-01-23 18:38:56 PST

Created attachment 184376 [details]
Patch take #2

Comment 8 James Craig 2013-01-23 18:41:09 PST

(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 9 WebKit Review Bot 2013-01-23 19:03:26 PST

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 10 Build Bot 2013-01-23 22:05:08 PST

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 11 Peter Beverloo (cr-android ews) 2013-01-23 22:28:42 PST

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

Comment 12 James Craig 2013-01-24 22:06:44 PST

Created attachment 184663 [details]
Patch #2

Comment 13 Peter Beverloo (cr-android ews) 2013-01-25 01:19:22 PST

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 14 WebKit Review Bot 2013-01-25 03:13:24 PST

Comment on attachment 184663 [details]
Patch #2

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

Comment 15 James Craig 2013-01-25 15:56:45 PST

Created attachment 184831 [details]
Patch #4

Attempting to resolve the chromium test run errors.

Comment 16 chris fleizach 2013-01-25 22:46:57 PST

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

Comment 17 James Craig 2013-01-25 23:22:00 PST

(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 18 Build Bot 2013-01-26 11:30:52 PST

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

Comment 19 James Craig 2013-01-26 16:09:10 PST

Created attachment 184889 [details]
Patch #5

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

Comment 20 James Craig 2013-01-26 16:12:22 PST

Created attachment 184890 [details]
Patch #5

Previous upload choked.

Comment 21 Dominic Mazzoni 2013-01-28 08:55:26 PST

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

Comment 22 WebKit Review Bot 2013-01-28 09:07:31 PST

Comment on attachment 184890 [details]
Patch #5

Clearing flags on attachment: 184890

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

Comment 23 WebKit Review Bot 2013-01-28 09:07:40 PST

All reviewed patches have been landed.  Closing bug.

Comment 24 Ryosuke Niwa 2013-01-29 21:31:07 PST

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

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