Bug 96173

Summary: REGRESSION (r127936): Multiple accessibility tests failing on Lion
Product: WebKit Reporter: mitz
Component: AccessibilityAssignee: Dominic Mazzoni <dmazzoni>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, dmazzoni, thorton, webkit.review.bot
Priority: P1 Keywords: LayoutTestFailure, MakingBotsRed, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r127946%20(650)/results.html
Attachments:
Description Flags
Patch none

mitz
Reported 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.
Attachments
Patch (2.42 KB, patch)
2012-09-09 14:19 PDT, Dominic Mazzoni 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dominic Mazzoni
Comment 1 2012-09-07 21:00:01 PDT
I'll look at these and either fix or revert shortly.
Dominic Mazzoni
Comment 2 2012-09-07 23:42:10 PDT
I took a look; no new bugs, just need to rebaseline some expectations.
Dominic Mazzoni
Comment 3 2012-09-07 23:58:57 PDT
Committed revision 127963.
Alexey Proskuryakov
Comment 4 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.
Dominic Mazzoni
Comment 5 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.
Dominic Mazzoni
Comment 6 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>
Dominic Mazzoni
Comment 7 2012-09-09 14:06:37 PDT
Fixed the text expectations only: Committed r127993: <http://trac.webkit.org/changeset/127993>
Dominic Mazzoni
Comment 8 2012-09-09 14:19:06 PDT
Created attachment 163013 [details] Patch
chris fleizach
Comment 9 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
Dominic Mazzoni
Comment 10 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;
chris fleizach
Comment 11 2012-09-10 09:42:15 PDT
Comment on attachment 163013 [details] Patch that seems like reasonable logic.
WebKit Review Bot
Comment 12 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>
WebKit Review Bot
Comment 13 2012-09-10 10:01:31 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.