AX: improve list heuristics (presentational use versus actual lists) Attaching a layout test that should better clarify what WebKit should exposed as a "list" to the accessibility APIs. The expected rolename "generic" of the non-list examples may need to change, but the rest is solid I believe.
<rdar://problem/17415885>
Created attachment 233587 [details] layout test
Created attachment 253470 [details] patch
Comment on attachment 253470 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=253470&action=review Test coverage is critical, because the accessibility code is complicated and much different from the normal DOM and rendering of these elements. > Source/WebCore/accessibility/AccessibilityList.cpp:104 > + Element* listItemElement = is<Element>(listItem->node()) ? downcast<Element>(listItem->node()) : nullptr; Not sure what the point of the is<Element> check here, because ... > Source/WebCore/accessibility/AccessibilityList.cpp:105 > + if (!listItemElement->beforePseudoElement()) here we would dereference the null pointer. I think this needs an additional null check.
Comment on attachment 253470 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=253470&action=review >> Source/WebCore/accessibility/AccessibilityList.cpp:105 >> + if (!listItemElement->beforePseudoElement()) > > here we would dereference the null pointer. I think this needs an additional null check. i didn't want to downcast to Element unless it was an Element. I guess that's taken care of by the downcast good catch. thanks
http://trac.webkit.org/changeset/184676
Comment on attachment 253470 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=253470&action=review > LayoutTests/accessibility/list-detection2-expected.txt:25 > + Strange that the "TEST COMPLETE" line came out before the results. Was that an error in my layout test?
(In reply to comment #7) > Comment on attachment 253470 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=253470&action=review > > > LayoutTests/accessibility/list-detection2-expected.txt:25 > > + > > Strange that the "TEST COMPLETE" line came out before the results. Was that > an error in my layout test? The before:: content image stuff didn't load early enough for the script to run so i had to run the test after the body finished loading
(In reply to comment #8) > The before:: content image stuff didn't load early enough for the script to > run so i had to run the test after the body finished loading Makes sense. Thanks for the additional info.
The new test fails on all non-Mac platforms: TEST COMPLETE -PASS: ul[role="list"] w/ explicit role and displayed inline -> list. -PASS: ul[role="list"] w/ explicit role but no markers -> list. -PASS: ul w/ default list markers -> list. -PASS: ol w/ default list counters -> list. -PASS: ul w/ list-style-image -> list. -PASS: ul w/ image content on ::before -> list. -PASS: ul w/ image content on inline ::before -> list. -PASS: ul w/ bullet content on ::before -> list. -PASS: ul w/ bullet content on inline ::before -> list. -PASS: ol w/ counter content on ::before -> list. -PASS: ol w/ counter content on inline ::before -> list. -PASS: ul w/ background image (NOT A LIST) -> group. -PASS: ul w/ background on ::before (NOT A LIST) -> group. -PASS: ul w/o explicit role and displayed inline, which defaults to no markers (NOT A LIST) -> group. -PASS: ol w/o explicit role and displayed inline, which defaults to no markers (NOT A LIST) -> group. -PASS: ul w/o explicit role and no markers (NOT A LIST) -> group. +FAIL: ul[role="list"] w/ explicit role and displayed inline -> . Expected: list. +FAIL: ul[role="list"] w/ explicit role but no markers -> . Expected: list. +FAIL: ul w/ default list markers -> . Expected: list. +FAIL: ol w/ default list counters -> . Expected: list. +FAIL: ul w/ list-style-image -> . Expected: list. +FAIL: ul w/ image content on ::before -> . Expected: list. +FAIL: ul w/ image content on inline ::before -> . Expected: list. +FAIL: ul w/ bullet content on ::before -> . Expected: list. +FAIL: ul w/ bullet content on inline ::before -> . Expected: list. +FAIL: ol w/ counter content on ::before -> . Expected: list. +FAIL: ol w/ counter content on inline ::before -> . Expected: list. +FAIL: ul w/ background image (NOT A LIST) -> . Expected: group. +FAIL: ul w/ background on ::before (NOT A LIST) -> . Expected: group. +FAIL: ul w/o explicit role and displayed inline, which defaults to no markers (NOT A LIST) -> . Expected: group. +FAIL: ol w/o explicit role and displayed inline, which defaults to no markers (NOT A LIST) -> . Expected: group. +FAIL: ul w/o explicit role and no markers (NOT A LIST) -> . Expected: group.
(In reply to comment #10) > The new test fails on all non-Mac platforms: > > TEST COMPLETE > -PASS: ul[role="list"] w/ explicit role and displayed inline -> list. > -PASS: ul[role="list"] w/ explicit role but no markers -> list. > -PASS: ul w/ default list markers -> list. > -PASS: ol w/ default list counters -> list. > -PASS: ul w/ list-style-image -> list. > -PASS: ul w/ image content on ::before -> list. > -PASS: ul w/ image content on inline ::before -> list. > -PASS: ul w/ bullet content on ::before -> list. > -PASS: ul w/ bullet content on inline ::before -> list. > -PASS: ol w/ counter content on ::before -> list. > -PASS: ol w/ counter content on inline ::before -> list. > -PASS: ul w/ background image (NOT A LIST) -> group. > -PASS: ul w/ background on ::before (NOT A LIST) -> group. > -PASS: ul w/o explicit role and displayed inline, which defaults to no > markers (NOT A LIST) -> group. > -PASS: ol w/o explicit role and displayed inline, which defaults to no > markers (NOT A LIST) -> group. > -PASS: ul w/o explicit role and no markers (NOT A LIST) -> group. > +FAIL: ul[role="list"] w/ explicit role and displayed inline -> . Expected: > list. > +FAIL: ul[role="list"] w/ explicit role but no markers -> . Expected: list. > +FAIL: ul w/ default list markers -> . Expected: list. > +FAIL: ol w/ default list counters -> . Expected: list. > +FAIL: ul w/ list-style-image -> . Expected: list. > +FAIL: ul w/ image content on ::before -> . Expected: list. > +FAIL: ul w/ image content on inline ::before -> . Expected: list. > +FAIL: ul w/ bullet content on ::before -> . Expected: list. > +FAIL: ul w/ bullet content on inline ::before -> . Expected: list. > +FAIL: ol w/ counter content on ::before -> . Expected: list. > +FAIL: ol w/ counter content on inline ::before -> . Expected: list. > +FAIL: ul w/ background image (NOT A LIST) -> . Expected: group. > +FAIL: ul w/ background on ::before (NOT A LIST) -> . Expected: group. > +FAIL: ul w/o explicit role and displayed inline, which defaults to no > markers (NOT A LIST) -> . Expected: group. > +FAIL: ol w/o explicit role and displayed inline, which defaults to no > markers (NOT A LIST) -> . Expected: group. > +FAIL: ul w/o explicit role and no markers (NOT A LIST) -> . Expected: group. Looking into it
http://trac.webkit.org/changeset/184721
*** Bug 170179 has been marked as a duplicate of this bug. ***