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: | Accessibility | Assignee: | 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
James Craig
2013-10-04 00:36:36 PDT
Created attachment 224138 [details]
patch
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.
Created attachment 224140 [details]
patch
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 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 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 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 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 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 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
(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. Created attachment 224220 [details]
patch for EWS
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. |