Bug 106243 – Need new accessibility layout test to verify levels of headings

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[James Craig]

new accessibility layout test to verify levels of headings: both native HTML (h1-h6) and WAI-ARIA ([role="heading"][aria-level="*"])

[James Craig]

Created attachment 182004 [details]
accessibility layout test case verifying AX representations of heading levels

[James Craig]

Created attachment 182261 [details]
layout test patch and changelog (unreviewed)

[WebKit Review Bot]

Comment on attachment 182261 [details]
layout test patch and changelog (unreviewed)

Attachment 182261 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15808222

New failing tests:
accessibility/heading-level.html

[chris fleizach]

Comment on attachment 182261 [details]
layout test patch and changelog (unreviewed)

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

test looks like its failing on chromium because they implement intValue() as 
void AccessibilityUIElement::intValueGetterCallback(CppVariant* result)
{
    result->set(accessibilityObject().valueForRange());
}

and we do something different.

I would skip this test in LayoutTests/platform/chromium/TestExpectations and make a bug for chromium to resolve this intValue() difference and assign to Dominic

> LayoutTests/ChangeLog:3
> +        ER: new accessibility layout test to verify levels of headings

ER is probably not appropriate, since it's just a layout test. Also capitalize beginning of bug

> LayoutTests/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

add some comment why this is needed

> LayoutTests/accessibility/heading-level-expected.txt:1
> +X

it'd be cool to hide all these X's once the test is done with something like

document.getElementById("test-div").style.display = none or something

> LayoutTests/accessibility/heading-level.html:14
> +<!-- explicit aria-level overrides on h1-h6 (withOUT explicit heading role declaration)  -->

i would reference the webkit bug that causes this to be broken

> LayoutTests/accessibility/heading-level.html:47
> +	for (var i=0, c=examples.length; i<c; i++){

use a space between terms 
i=0 --> i = 0
i<c --> i  < c

space between ){

> LayoutTests/accessibility/heading-level.html:55
> +			result.innerText += "PASS: level is " + axElement.intValue + ".\n";

it looks like tabs are being used. 
webkit prefers only spaces instead of tabs (4 spaces per indent)

[James Craig]

Created attachment 182400 [details]
layout test patch, chromium expectation mod, and changelog (reviewed by CF)

[chris fleizach]

Comment on attachment 182400 [details]
layout test patch, chromium expectation mod, and changelog (reviewed by CF)

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

otherwise looks good

> LayoutTests/ChangeLog:3
> +        New layout test to verify heading levels on implicit h1-h6 and explicit @aria-level.

this line should go below the "Reviewed by" line, and in its place should be the title of the bug

don't put my name in the Reviewed field. that will be auto filled by the commit queue

[James Craig]

Created attachment 182412 [details]
changelog modification from previous patch

[chris fleizach]

Comment on attachment 182412 [details]
changelog modification from previous patch

Looks good. Lets make sure the bots are happy

[WebKit Review Bot]

Comment on attachment 182412 [details]
changelog modification from previous patch

Rejecting attachment 182412 [details] from commit-queue.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', u'--status-host=queues.webkit.org', ..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

/mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/15803640

[chris fleizach]

Comment on attachment 182412 [details]
changelog modification from previous patch

you need to leave the Reviewed by OOPS line in there so it can auto fill it in

[James Craig]

Created attachment 182446 [details]
Same patch plus "reviewed by" placeholder for the bot. I, for one, welcome our robot overlords.

[WebKit Review Bot]

Comment on attachment 182446 [details]
Same patch plus "reviewed by" placeholder for the bot. I, for one, welcome our robot overlords.

Clearing flags on attachment: 182446

Committed r139534: <http://trac.webkit.org/changeset/139534>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.