Bug 131041 - Regression: AX: list heuristics sometimes determined as presentational even when explicit roles applied
Summary: Regression: AX: list heuristics sometimes determined as presentational even w...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks: 131025
  Show dependency treegraph
 
Reported: 2014-04-01 09:14 PDT by James Craig
Modified: 2014-04-09 16:52 PDT (History)
11 users (show)

See Also:


Attachments
test case (last list in page) (1.00 KB, text/html)
2014-04-01 09:14 PDT, James Craig
no flags Details
screen shot showing computed group role (93.50 KB, image/png)
2014-04-01 09:24 PDT, James Craig
no flags Details
patch (3.34 KB, patch)
2014-04-04 09:58 PDT, chris fleizach
mario: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (491.83 KB, application/zip)
2014-04-04 11:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (543.52 KB, application/zip)
2014-04-04 11:59 PDT, Build Bot
no flags Details
patch (3.30 KB, patch)
2014-04-05 17:31 PDT, chris fleizach
mario: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (564.54 KB, application/zip)
2014-04-05 18:35 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description James Craig 2014-04-01 09:14:52 PDT
Created attachment 228289 [details]
test case (last list in page)

AX: list heuristics sometimes determined as presentational even when explicit roles applied

In the test case, the first list is a standard list, and the second is correctly determined as presentational, but the last list is incorrectly determined as presentational. It should retain the list role because the element uses explicit role values.

(This bug may be masking another issue reported in bug 131025. That one is not currently reproducible, but I'm not convinced it's gone.)
Comment 1 Radar WebKit Bug Importer 2014-04-01 09:15:46 PDT
<rdar://problem/16486230>
Comment 2 James Craig 2014-04-01 09:24:15 PDT
Created attachment 228292 [details]
screen shot showing computed group role
Comment 3 James Craig 2014-04-01 18:39:31 PDT
This is a regression from the recent work on list heuristics.
Comment 4 chris fleizach 2014-04-04 09:58:32 PDT
Created attachment 228598 [details]
patch
Comment 5 Build Bot 2014-04-04 11:02:53 PDT
Comment on attachment 228598 [details]
patch

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

New failing tests:
accessibility/list-detection.html
Comment 6 Build Bot 2014-04-04 11:25:46 PDT
Comment on attachment 228598 [details]
patch

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

New failing tests:
accessibility/list-detection.html
Comment 7 Build Bot 2014-04-04 11:25:50 PDT
Created attachment 228610 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2014-04-04 11:59:28 PDT
Comment on attachment 228598 [details]
patch

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

New failing tests:
platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
accessibility/list-detection.html
platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
platform/mac/fast/scrolling/scroll-div-latched-mainframe.html
Comment 9 Build Bot 2014-04-04 11:59:32 PDT
Created attachment 228611 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 10 chris fleizach 2014-04-05 17:31:33 PDT
Created attachment 228690 [details]
patch

The first patch was a false start. The real problem was that display:table was being used on the <ul> which inserts anonymous RenderTableCells, which cause the children of the list to be "wrong".

I think the right fix is to ignore RenderTableCells without node()s, which should indicate they've been created as a side effect of display:table and do not add semantic information.
Comment 11 WebKit Commit Bot 2014-04-05 17:33:54 PDT
Attachment 228690 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityTableCell.cpp:67:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Build Bot 2014-04-05 18:35:03 PDT
Comment on attachment 228690 [details]
patch

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

New failing tests:
platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
media/W3C/audio/canPlayType/canPlayType_application_octet_stream_with_codecs_1.html
platform/mac/fast/scrolling/scroll-div-latched-mainframe.html
Comment 13 Build Bot 2014-04-05 18:35:07 PDT
Created attachment 228692 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 14 chris fleizach 2014-04-05 19:13:46 PDT
(In reply to comment #11)
> Attachment 228690 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/accessibility/AccessibilityTableCell.cpp:67:  Multi line control clauses should use braces.  [whitespace/braces] [4]
> Total errors found: 1 in 5 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

This seems wrong...
Comment 15 chris fleizach 2014-04-05 19:14:17 PDT
(In reply to comment #12)
> (From update of attachment 228690 [details])
> Attachment 228690 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/4662735514107904
> 
> New failing tests:
> platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
> platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
> media/W3C/audio/canPlayType/canPlayType_application_octet_stream_with_codecs_1.html
> platform/mac/fast/scrolling/scroll-div-latched-mainframe.html

these all seem unrelated
Comment 16 chris fleizach 2014-04-07 22:58:37 PDT
(In reply to comment #10)
> Created an attachment (id=228690) [details]
> patch
> 
> The first patch was a false start. The real problem was that display:table was being used on the <ul> which inserts anonymous RenderTableCells, which cause the children of the list to be "wrong".
> 
> I think the right fix is to ignore RenderTableCells without node()s, which should indicate they've been created as a side effect of display:table and do not add semantic information.

Mario, can you review this one again. Thanks
Comment 17 Mario Sanchez Prada 2014-04-09 07:58:09 PDT
(In reply to comment #14)
> (In reply to comment #11)
> > Attachment 228690 [details] [details] did not pass style-queue:
> > 
> > 
> > ERROR: Source/WebCore/accessibility/AccessibilityTableCell.cpp:67:  Multi line control clauses should use braces.  [whitespace/braces] [4]
> > Total errors found: 1 in 5 files
> > 
> > 
> > If any of these errors are false positives, please file a bug against check-webkit-style.
> 
> This seems wrong...

I agree


(In reply to comment #15)
> (In reply to comment #12)
> > (From update of attachment 228690 [details] [details])
> > Attachment 228690 [details] [details] did not pass mac-wk2-ews (mac-wk2):
> > Output: http://webkit-queues.appspot.com/results/4662735514107904
> > 
> > New failing tests:
> > platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
> > platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
> > media/W3C/audio/canPlayType/canPlayType_application_octet_stream_with_codecs_1.html
> > platform/mac/fast/scrolling/scroll-div-latched-mainframe.html
> 
> these all seem unrelated

I agree too
Comment 18 chris fleizach 2014-04-09 16:52:22 PDT
http://trac.webkit.org/changeset/167052