Bug 158876 - AX: HTML indeterminate IDL attribute not mapped to checkbox value=2 for native checkboxes
Summary: AX: HTML indeterminate IDL attribute not mapped to checkbox value=2 for nativ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-17 10:19 PDT by chris fleizach
Modified: 2016-06-17 18:19 PDT (History)
9 users (show)

See Also:


Attachments
patch (5.36 KB, patch)
2016-06-17 11:44 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (5.44 KB, patch)
2016-06-17 17:02 PDT, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2016-06-17 10:19:40 PDT
Indeterminate not working on native checkboxes
Comment 1 chris fleizach 2016-06-17 10:19:54 PDT
<rdar://problem/26842619>
Comment 2 Radar WebKit Bug Importer 2016-06-17 10:20:23 PDT
<rdar://problem/26864598>
Comment 3 chris fleizach 2016-06-17 11:44:59 PDT
Created attachment 281571 [details]
patch
Comment 4 Joanmarie Diggs 2016-06-17 16:29:04 PDT
Comment on attachment 281571 [details]
patch

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

> LayoutTests/accessibility/checkbox-mixed-value.html:27
> +              debug(element.stringValue + "\n");

Please use (at least*) AccessibilityUIElement::isIndeterminate(). On my platform, exposure of this property is not through string value, but through state.

* "at least" because I think that is sufficient -- unless you want to not just verify not indeterminate, but also not checked.
Comment 5 chris fleizach 2016-06-17 16:30:44 PDT
Comment on attachment 281571 [details]
patch

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

>> LayoutTests/accessibility/checkbox-mixed-value.html:27
>> +              debug(element.stringValue + "\n");
> 
> Please use (at least*) AccessibilityUIElement::isIndeterminate(). On my platform, exposure of this property is not through string value, but through state.
> 
> * "at least" because I think that is sufficient -- unless you want to not just verify not indeterminate, but also not checked.

In this case we want to test that value ends up producing a 2 instead of 0. Maybe I should just move this to the Mac platform in that case? What do you think
Comment 6 Joanmarie Diggs 2016-06-17 16:49:14 PDT
(In reply to comment #5)
> Comment on attachment 281571 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281571&action=review
> 
> >> LayoutTests/accessibility/checkbox-mixed-value.html:27
> >> +              debug(element.stringValue + "\n");
> > 
> > Please use (at least*) AccessibilityUIElement::isIndeterminate(). On my platform, exposure of this property is not through string value, but through state.
> > 
> > * "at least" because I think that is sufficient -- unless you want to not just verify not indeterminate, but also not checked.
> 
> In this case we want to test that value ends up producing a 2 instead of 0.
> Maybe I should just move this to the Mac platform in that case? What do you
> think

I think the test belongs cross-platform if possible. Here's your platform's method:

bool AccessibilityUIElement::isIndeterminate() const
{
    BOOL result = NO;
    BEGIN_AX_OBJC_EXCEPTIONS
    id value = [m_element accessibilityAttributeValue:NSAccessibilityValueAttribute];
    if ([value isKindOfClass:[NSNumber class]])
        result = [value intValue] == 2;
    END_AX_OBJC_EXCEPTIONS
    return result;
}

Does that not verify that the value ends up producing a 2 instead of 0?
Comment 7 chris fleizach 2016-06-17 16:51:29 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 281571 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=281571&action=review
> > 
> > >> LayoutTests/accessibility/checkbox-mixed-value.html:27
> > >> +              debug(element.stringValue + "\n");
> > > 
> > > Please use (at least*) AccessibilityUIElement::isIndeterminate(). On my platform, exposure of this property is not through string value, but through state.
> > > 
> > > * "at least" because I think that is sufficient -- unless you want to not just verify not indeterminate, but also not checked.
> > 
> > In this case we want to test that value ends up producing a 2 instead of 0.
> > Maybe I should just move this to the Mac platform in that case? What do you
> > think
> 
> I think the test belongs cross-platform if possible. Here's your platform's
> method:
> 
> bool AccessibilityUIElement::isIndeterminate() const
> {
>     BOOL result = NO;
>     BEGIN_AX_OBJC_EXCEPTIONS
>     id value = [m_element
> accessibilityAttributeValue:NSAccessibilityValueAttribute];
>     if ([value isKindOfClass:[NSNumber class]])
>         result = [value intValue] == 2;
>     END_AX_OBJC_EXCEPTIONS
>     return result;
> }
> 
> Does that not verify that the value ends up producing a 2 instead of 0?

Good point. That should do it
Comment 8 Joanmarie Diggs 2016-06-17 17:00:17 PDT
BTW, from a quick glance at window's DRT (are they still using that?), AccessibilityUIElement::isIndeterminate() is not implemented. If they are still using DRT, skipping for Windows is probably going to be needed.
Comment 9 chris fleizach 2016-06-17 17:02:38 PDT
Created attachment 281598 [details]
patch
Comment 10 Joanmarie Diggs 2016-06-17 17:51:05 PDT
Comment on attachment 281598 [details]
patch

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

> LayoutTests/accessibility/checkbox-mixed-value.html:27
> +              debug("Indeterminate status: " + element.isIndeterminate + "\n");

Thanks!
Comment 11 WebKit Commit Bot 2016-06-17 18:19:15 PDT
Comment on attachment 281598 [details]
patch

Clearing flags on attachment: 281598

Committed r202194: <http://trac.webkit.org/changeset/202194>
Comment 12 WebKit Commit Bot 2016-06-17 18:19:19 PDT
All reviewed patches have been landed.  Closing bug.