Bug 96173 - REGRESSION (r127936): Multiple accessibility tests failing on Lion
Summary: REGRESSION (r127936): Multiple accessibility tests failing on Lion
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Dominic Mazzoni
URL: http://build.webkit.org/results/Apple...
Keywords: LayoutTestFailure, MakingBotsRed, Regression
Depends on:
Blocks:
 
Reported: 2012-09-07 20:19 PDT by mitz
Modified: 2012-09-10 10:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.42 KB, patch)
2012-09-09 14:19 PDT, Dominic Mazzoni
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2012-09-07 20:19:01 PDT
After <http://trac.webkit.org/r127936>, the fix for bug 94870, multiple accessibility tests began to fail. See the URL for example.
Comment 1 Dominic Mazzoni 2012-09-07 21:00:01 PDT
I'll look at these and either fix or revert shortly.
Comment 2 Dominic Mazzoni 2012-09-07 23:42:10 PDT
I took a look; no new bugs, just need to rebaseline some expectations.
Comment 3 Dominic Mazzoni 2012-09-07 23:58:57 PDT
Committed revision 127963.
Comment 4 Alexey Proskuryakov 2012-09-08 19:35:26 PDT
In addition to changing expectations, you changed a test:

> Updating one test (update-children-when-aria-role-changes.html) that 
> incorrectly assumed a node with an aria-lable would be ignored. Used title 
> instead of aria-label. 

This kind of change requires review.
Comment 5 Dominic Mazzoni 2012-09-09 12:04:02 PDT
(In reply to comment #4)
> In addition to changing expectations, you changed a test:
> 
> > Updating one test (update-children-when-aria-role-changes.html) that 
> > incorrectly assumed a node with an aria-lable would be ignored. Used title 
> > instead of aria-label. 
> 
> This kind of change requires review.

Sorry. Fixing this now - I'll revert the offending change, fix just the expectations, then upload a patch to fix the test for review.
Comment 6 Dominic Mazzoni 2012-09-09 13:52:12 PDT
Reverted r127963 for reason:

Inappropriately modified test, and not just expectations, without review

Committed r127992: <http://trac.webkit.org/changeset/127992>
Comment 7 Dominic Mazzoni 2012-09-09 14:06:37 PDT
Fixed the text expectations only:

Committed r127993: <http://trac.webkit.org/changeset/127993>
Comment 8 Dominic Mazzoni 2012-09-09 14:19:06 PDT
Created attachment 163013 [details]
Patch
Comment 9 chris fleizach 2012-09-10 08:59:47 PDT
Comment on attachment 163013 [details]
Patch

do you know what kind of role a <span> becomes if it's not ignored all of a sudden. i want to make sure we won't start exposing AXUnknown elements if someone puts an aria-label on a <span> without a role mapping
Comment 10 Dominic Mazzoni 2012-09-10 09:17:14 PDT
(In reply to comment #9)
> (From update of attachment 163013 [details])
> do you know what kind of role a <span> becomes if it's not ignored all of a sudden. i want to make sure we won't start exposing AXUnknown elements if someone puts an aria-label on a <span> without a role mapping

It will be AXGroup. I tested it to confirm, but the logic is at the very bottom of determineAccessibilityRole:

if (supportsARIAAttributes())
    return GroupRole;
Comment 11 chris fleizach 2012-09-10 09:42:15 PDT
Comment on attachment 163013 [details]
Patch

that seems like reasonable logic.
Comment 12 WebKit Review Bot 2012-09-10 10:01:27 PDT
Comment on attachment 163013 [details]
Patch

Clearing flags on attachment: 163013

Committed r128072: <http://trac.webkit.org/changeset/128072>
Comment 13 WebKit Review Bot 2012-09-10 10:01:31 PDT
All reviewed patches have been landed.  Closing bug.