Bug 96735

Summary: AX: A few control types are returning the wrong answer for isReadOnly
Product: WebKit Reporter: Dominic Mazzoni <dmazzoni>
Component: AccessibilityAssignee: Dominic Mazzoni <dmazzoni>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Dominic Mazzoni 2012-09-14 01:43:33 PDT
When isReadOnly was refactored from AccessibilityRenderObject to AccessibilityNodeObject, it changed the answer to isReadOnly for a few input types.

To clarify: In this context, read only means that element cannot be edited like text. It has nothing to do with whether a control's value can be modified - that's "unavailable" or "disabled".

This can't be tested easily on Mac; it doesn't look like Mac exposes isReadOnly directly.
Comment 1 Dominic Mazzoni 2012-09-14 01:54:23 PDT
Created attachment 164072 [details]
Patch
Comment 2 chris fleizach 2012-09-18 17:46:27 PDT
Comment on attachment 164072 [details]
Patch

isReadOnly is used to determine if canSetValueAttribute is true or false. For many input types, AXValue is writable (on mac) like checkboxes, radio, etc, so I think this will break behavior
Comment 3 Dominic Mazzoni 2012-09-19 11:16:49 PDT
Created attachment 164758 [details]
Patch
Comment 4 Dominic Mazzoni 2012-09-19 11:19:31 PDT
(In reply to comment #2)
> (From update of attachment 164072 [details])
> isReadOnly is used to determine if canSetValueAttribute is true or false. For many input types, AXValue is writable (on mac) like checkboxes, radio, etc, so I think this will break behavior

Checkboxes, radio did not have AXValue writable previous - but they should be for a slider or text field.

I'm pretty sure this change is just restoring the previous behavior before the latest axnode refactor. I added a Mac test to make it clear what the end result is, and compared those results to the stable build of Safari to confirm that the results are identical.
Comment 5 chris fleizach 2012-09-19 11:31:04 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 164072 [details] [details])
> > isReadOnly is used to determine if canSetValueAttribute is true or false. For many input types, AXValue is writable (on mac) like checkboxes, radio, etc, so I think this will break behavior
> 
> Checkboxes, radio did not have AXValue writable previous - but they should be for a slider or text field.
> 
> I'm pretty sure this change is just restoring the previous behavior before the latest axnode refactor. I added a Mac test to make it clear what the end result is, and compared those results to the stable build of Safari to confirm that the results are identical.

Thanks for looking into this.

I checked on Mac, combo boxes should be writeable and not read only (because you can type into the combo box)

it looks like list boxes AXSelectedChildren should be writeable (but not its AXValue)
Comment 6 Dominic Mazzoni 2012-09-19 11:47:14 PDT
(In reply to comment #5)
> I checked on Mac, combo boxes should be writeable and not read only (because you can type into the combo box)

Are you sure? This is just for a <select> control, you can't type other than type-ahead. There's no true combo box in html.

Either way, is it okay if we land this patch first in order to restore the previous behavior, and then open a new bug with the changes? This will fix a regression in Chromium, and I'd rather get us back to a known-good state first before trying something new.

If we make changes to isReadOnly for form controls I may have to make some Chromium changes.

- Dominic
Comment 7 WebKit Review Bot 2012-09-19 12:54:56 PDT
Comment on attachment 164758 [details]
Patch

Clearing flags on attachment: 164758

Committed r129036: <http://trac.webkit.org/changeset/129036>
Comment 8 WebKit Review Bot 2012-09-19 12:54:59 PDT
All reviewed patches have been landed.  Closing bug.