Bug 131041

Summary:

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

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:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

131025

Attachments:

Description	Flags
test case (last list in page)	none
screen shot showing computed group role	none
patch	mario: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2	none
patch	mario: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2	none

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