Bug 122320

Summary: AX: WebKit needs heuristics to differentiate lists used for layout from semantic data lists, similar to the heuristics for layout tables versus data tables.
Product: WebKit Reporter: James Craig <jcraig>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, buildbot, cfleizach, commit-queue, dmazzoni, jdiggs, mario, rniwa, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch
mario: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
none
patch for EWS none

Description James Craig 2013-10-04 00:36:36 PDT
AX: WebKit needs heuristics to differentiate lists used for layout from semantic data lists, similar to the heuristics for layout tables versus data tables, and weeding out spacer images. Extra lists cause VoiceOver to speak stuff like "list, one item" and sometime group things erroneously or speak labels twice.

Some potential criteria:
1. If it uses ARIA roles, keep it as a semantic list.
2. If it displays generated list markers (like bullets or numbers), keep it as a semantic list. 
3. If it contains well-formed nested lists, assume they are all semantic lists.
4. If the list only contains a single item, presume it's a layout list.
7. If it ~acts like a large scale container (e.g. contains a large amount of distinct content elements), assume it's a layout list. (This will be the tricky one.)

Moreā€¦
Comment 1 Radar WebKit Bug Importer 2013-10-04 00:37:07 PDT
<rdar://problem/15150097>
Comment 2 chris fleizach 2014-02-13 18:00:01 PST
Created attachment 224138 [details]
patch
Comment 3 WebKit Commit Bot 2014-02-13 18:01:48 PST
Attachment 224138 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityList.cpp:149:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 chris fleizach 2014-02-13 18:10:15 PST
Created attachment 224140 [details]
patch
Comment 5 Build Bot 2014-02-13 19:32:32 PST
Comment on attachment 224140 [details]
patch 

Attachment 224140 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6052477241655296

New failing tests:
accessibility/aria-roles.html
Comment 6 Build Bot 2014-02-13 19:32:35 PST
Created attachment 224146 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 7 Build Bot 2014-02-14 01:44:24 PST
Comment on attachment 224140 [details]
patch 

Attachment 224140 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5354837650702336

New failing tests:
accessibility/aria-roles.html
Comment 8 Build Bot 2014-02-14 01:44:27 PST
Created attachment 224179 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 9 Mario Sanchez Prada 2014-02-14 02:26:26 PST
Comment on attachment 224140 [details]
patch 

View in context: https://bugs.webkit.org/attachment.cgi?id=224140&action=review

The change looks good to me. My only comments/suggestions is to remove the PLATFORM(MAC) guard and make the tests cross platform, since I believe this is a change other ports can benefit of as well.

As for the failing tests in the Mac EWS, it seems to me that the problem is that the test aria-roles.html needs to be changed because the following snippet does not match the heuristic to determine that something is a list:

  <!--List-->
  <div class="newRole">
    <p>The following should be a list:</p>
    <p><span tabindex="0" role="list" id="ariaList">X</span></p>
    <p>Actual list:</p>
    <ul id="realList">
        <li>Broccoli</li>
        <li>Beets</li>
    </ul>
    <span id="resultList"></span>
    <script>
        validateRole(document.getElementById('ariaList'),
            document.getElementById('realList'),
            document.getElementById('resultList'));
    </script>
  </div>

The problem is that <span tabindex="0" role="list" id="ariaList">X</span> is no longer going to be identified as a list so I'd say either this whole snippet of code should be removed (it's tested anyway by list-detection.html, and in a better way) or adapted.

But other than that I think the patch is fine. Setting r+ anyway, but please consider these comments before landing

> Source/WebCore/accessibility/AccessibilityList.cpp:120
> +#if PLATFORM(MAC)

I think this change could be done as a cross-platform one

> LayoutTests/ChangeLog:9
> +        * platform/mac/accessibility/list-detection-expected.txt: Added.
> +        * platform/mac/accessibility/list-detection.html: Added.

This tests could be moved up to LayoutTests/accessibility if the change is made cross platform
Comment 10 Build Bot 2014-02-14 02:29:18 PST
Comment on attachment 224140 [details]
patch 

Attachment 224140 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4667337135357952

New failing tests:
accessibility/aria-roles.html
Comment 11 Build Bot 2014-02-14 02:29:20 PST
Created attachment 224184 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 12 chris fleizach 2014-02-14 08:43:03 PST
(In reply to comment #9)
> (From update of attachment 224140 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224140&action=review

Thanks. Will take care of these suggestions.
Comment 13 chris fleizach 2014-02-14 08:58:25 PST
Created attachment 224220 [details]
patch for EWS
Comment 14 chris fleizach 2014-02-14 09:47:52 PST
http://trac.webkit.org/changeset/164107
Comment 15 James Craig 2023-10-24 14:45:03 PDT
The original test case is here:
https://bug-134187-attachments.webkit.org/attachment.cgi?id=233587

Posting here in case I follow a git blame here again.