Bug 86949 - AX: A disabled select element should not be exposed as focusable
Summary: AX: A disabled select element should not be exposed as focusable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dominic Mazzoni
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-19 15:25 PDT by Dominic Mazzoni
Modified: 2012-05-21 11:42 PDT (History)
6 users (show)

See Also:


Attachments
Patch (7.76 KB, patch)
2012-05-21 08:38 PDT, Dominic Mazzoni
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (479.76 KB, application/zip)
2012-05-21 09:05 PDT, WebKit Review Bot
no flags Details
Patch (6.38 KB, patch)
2012-05-21 09:42 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 Dominic Mazzoni 2012-05-19 15:25:47 PDT
Test html: <select disabled><option>1<option>2</select>

The AccessibilityObject associated with the control has role PopUpButtonRole, and correctly returns false for isEnabled(), but incorrectly returns true for canSetFocusAttribute() even though it can't be focused.

The other form controls I checked behave correctly - if isEnabled() is false, canSetFocusAttribute() returns false. Should be a simple fix - it looks like AccessibilityMenuList.h has canSetFocusAttribute() always return true, but actually it should return false if the control isn't enabled.
Comment 1 Alice Boxhall 2012-05-20 21:14:22 PDT
I wonder whether this is related to bug 76119.
Comment 2 Dominic Mazzoni 2012-05-21 08:38:56 PDT
Created attachment 143041 [details]
Patch
Comment 3 chris fleizach 2012-05-21 09:00:30 PDT
Comment on attachment 143041 [details]
Patch

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

you have some failures in your tests

> Source/WebCore/accessibility/AccessibilityListBox.cpp:74
> +    return static_cast<Element*>(node())->isEnabledFormControl();

might be nice to add a comment why the default AccessibilityRenderObject::canSetFocus does not work for this kind of control
Comment 4 WebKit Review Bot 2012-05-21 09:05:08 PDT
Comment on attachment 143041 [details]
Patch

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

New failing tests:
accessibility/disabled-controls-not-focusable.html
Comment 5 WebKit Review Bot 2012-05-21 09:05:12 PDT
Created attachment 143044 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 6 Dominic Mazzoni 2012-05-21 09:41:39 PDT
(In reply to comment #3)
> might be nice to add a comment why the default AccessibilityRenderObject::canSetFocus does not work for this kind of control

You're right - the default implementation works fine for ListBox (but not for MenuList). A better fix is to just remove the override.

Test expectations should be correct now.
Comment 7 Dominic Mazzoni 2012-05-21 09:42:51 PDT
Created attachment 143051 [details]
Patch
Comment 8 chris fleizach 2012-05-21 10:43:10 PDT
Comment on attachment 143051 [details]
Patch

looks good
Comment 9 WebKit Review Bot 2012-05-21 11:42:15 PDT
Comment on attachment 143051 [details]
Patch

Clearing flags on attachment: 143051

Committed r117803: <http://trac.webkit.org/changeset/117803>
Comment 10 WebKit Review Bot 2012-05-21 11:42:20 PDT
All reviewed patches have been landed.  Closing bug.