348053 - implement accessible objects for xforms select controls

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Moving here related comments from bug 337690.

 ------- Comment #50 From [e-mail]  2006-08-07 13:48 PDT  [reply] -------

I don't know if these will present a problem or not.  I guess it depends on how
you will get the values.  Just keep in mind that xf:label and xf:value can
contain xf:output's.  According to the spec, xf:value can contain almost
anything and be valid, but xf:output is the only thing I've ever heard of as
being used as a child of xf:value.


------- Comment #51 From Aaron Leventhal 2006-08-07 14:02 PDT [reply] -------

(In reply to comment #50)
> I don't know if these will present a problem or not.  I guess it depends on how
> you will get the values.  Just keep in mind that xf:label and xf:value can
> contain xf:output's.  According to the spec, xf:value can contain almost
> anything and be valid, but xf:output is the only thing I've ever heard of as
> being used as a child of xf:value.
> 
As long as the label is rendered and has an accessible subtree we can point to
it using the labelledby accessible relation. The accessible name can be an
attempt to stringize it. The AT sees the item get focus and checks both, then
decides whether to use the subtree pointed to by the relation or the accessible
name.

As far as value we don't have anything like a valuefor relation.

Comment 1

[Aaron Leventhal]

The up/down arrow keys are doing double duty in terms of navigating the caret and changing item selection.

Is that issue already filed?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> The up/down arrow keys are doing double duty in terms of navigating the caret
> and changing item selection.
> 
> Is that issue already filed?
> 

Already yes :), bug 348867. Thank you for catching.

Comment 3

[alexander :surkov (:asurkov)]

Aaron Reed's comment from bug 337690 comment 65:

Just thought of something...should nsXFormsAccessible::GetState have a 'out of
range' state?  Or is that something that you are going to add later when you
add support for select, select1 and range?

Comment 4

[Aaron Leventhal]

Use STATE_INVALID for out-of-range.

Also, for a range we do have the ability to state the min and max value, via nsIAccessibleValue. So if something is STATE_INVALID the AT can check to see if it's outside of the min/max value.

Not sure how out of range works with a select/select1.

Comment 5

[aaronr]

(In reply to comment #4)
> Use STATE_INVALID for out-of-range.
> 
> Also, for a range we do have the ability to state the min and max value, via
> nsIAccessibleValue. So if something is STATE_INVALID the AT can check to see if
> it's outside of the min/max value.
> 
> Not sure how out of range works with a select/select1.
> 

If the select or select1 is bound to an instance data node that has a value that  is not in the item list, then the control enters the 'out of range' state.

Comment 6

[alexander :surkov (:asurkov)]

Here I like to implement accessible objects for thouse selects that are represented by native control like select/listbox and etc. I'll file another bugs for full selects and xhtml minimal select1. Marking blocking from bug 355540 since I want to make acessible objects for those selects as containers.

Comment 7

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 8

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 9

[Aaron Leventhal]

Comment on attachment 244821 [details] [diff] [review]
patch2

Is this just for select or also for select1?

Is there supposed to be a follow up patch to this? Or does this somehow magically deal with the child options?

The structure we provide needs to be similar to what we do for HTML combo boxes and lists.

Comment 10

[alexander :surkov (:asurkov)]

(In reply to comment #9)
> (From update of attachment 244821 [details] [diff] [review] [edit])
> Is this just for select or also for select1?

This is for select and select1. For those selects controls that are implemented by using native widgets. These are:
1) combobox representation: select1[appearance='minimal'] for xul
2) listbox representation: select1[appearance='compact'] and select[appearance='compact'] for xhtml and xul

> 
> Is there supposed to be a follow up patch to this? Or does this somehow
> magically deal with the child options?

Actually xforms item/choices element are hidden and are used only to build native widget. Therefore I guess I have only unique way is to allow native widget to be accessible and make select/select1 elements as containers.

Comment 11

[Aaron Leventhal]

(In reply to comment #10)
> Therefore I guess I have only unique way is to allow native
> widget to be accessible and make select/select1 elements as containers.

So what will the final hierarchy look like? Shouldn't it all be done with one patch? I'd like to see it working with Window-Eyes or JAWS.

Comment 12

[alexander :surkov (:asurkov)]

(In reply to comment #11)
> (In reply to comment #10)
> > Therefore I guess I have only unique way is to allow native
> > widget to be accessible and make select/select1 elements as containers.
> 
> So what will the final hierarchy look like?

For example,
<xforms:select appearance="compact">
  <html:select>
    <html:option/>
  </html:select>
</xforms:select>

or 
<xforms:select1 appearance="minimal">
  <xul:menulist>
    <xul:menupopup>
      <xul:menuitem/>
    </xul:menupopup>
    <xul:dropmarker/>
  </xul:menulist>
</xforms:select>

> Shouldn't it all be done with one patch?

Controls of this sort are supported in this patch. I don't assume no more patches for them. Other types of select controls are supposed to be implemented in bug 358713 and bug 358714.

> I'd like to see it working with Window-Eyes or JAWS.

Since xforms select controls expose anonymous nodes (html:select, xul:listbox, xul:menulist) then screen readers should work fine with them.

Comment 13

[Aaron Leventhal]

If you post a testcase I will build and look at it with JAWS and Window-Eyes.

In terms of final hierarchy I meant the MSAA hierarchy.

Comment 14

[alexander :surkov (:asurkov)]

Attachment: test

Select @appearance="compact" should be accessible.

Comment 15

[alexander :surkov (:asurkov)]

Attachment: xul test

Comment 16

[Aaron Leventhal]

Comment on attachment 244821 [details] [diff] [review]
patch2

r+ but we need to file bugs for the follow polish issues:

1) GetChildAtPoint is not working to get from the select into the option children
2) The checkboxes don't appear to be attached to their parent. I can't use MSAA Accessible Explorer to get to them.
3) The checkboxes don't appear to have an accessible name
4) I'm not sure that the extra groupbox is necessary for the compact presentation of the select. For the checkboxes it does make sense, but for the compact (list) appearance it seems like users will be confused if they're told their is a groupbox there. I know I told you to use ROLE_GROUPING, but do we need an accessible object for that item at all?
5) There is a focus event missing in the compact (list) case, when you first tab to into the list. In a normal list, you end up with 2 focus events in MSAA -- the first one goes to the list, and the second one goes to the current item. If the second focus event doesn't happen, the  screen reader does not announce the item which is landed on.

Comment 17

[alexander :surkov (:asurkov)]

(In reply to comment #16)

> 1) GetChildAtPoint is not working to get from the select into the option
> children
> 2) The checkboxes don't appear to be attached to their parent. I can't use MSAA
> Accessible Explorer to get to them.
> 3) The checkboxes don't appear to have an accessible name

Are these problem valid for original xhtml controls or xhtml controls inside xfroms controls only?

Comment 18

[Aaron Leventhal]

As far as I know, those problems exist only inside xforms.

Comment 19

[[e-mail]]

Comment on attachment 244821 [details] [diff] [review]
patch2

>+   /** Used for select and select1 that are implemented by host document native
>+    * widget using */
Comment doesn't make sense.

>-    <implementation implements="nsIXFormsUIWidget, nsIXFormsNSEditableElement">
>+    <implementation implements="nsIXFormsNSEditableElement, nsIAccessibleProvider">
nsIXFormsUIWidget seems to have got lost?

sr=me with these fixed.

Comment 20

[alexander :surkov (:asurkov)]

(In reply to comment #19)
> (From update of attachment 244821 [details] [diff] [review] [edit])
> >+   /** Used for select and select1 that are implemented by host document native
> >+    * widget using */
> Comment doesn't make sense.

Why? Every XForms contstants of nsIAccessibleProvider has similiar comment.

> >-    <implementation implements="nsIXFormsUIWidget, nsIXFormsNSEditableElement">
> >+    <implementation implements="nsIXFormsNSEditableElement, nsIAccessibleProvider">
> nsIXFormsUIWidget seems to have got lost?

No, nsIXFormsUIWidget stuf is inherited from base widget.

Comment 21

[[e-mail]]

(In reply to comment #20)
>(In reply to comment #19)
>>(From update of attachment 244821 [details] [diff] [review] [edit] [edit])
>>>+   /** Used for select and select1 that are implemented by host document native
>>>+    * widget using */
>>Comment doesn't make sense.
>Why?
Well, it ends with the word "using" for a start.

>>>-    <implementation implements="nsIXFormsUIWidget, nsIXFormsNSEditableElement">
>>>+    <implementation implements="nsIXFormsNSEditableElement, nsIAccessibleProvider">
>> nsIXFormsUIWidget seems to have got lost?
>No, nsIXFormsUIWidget stuf is inherited from base widget.
Oops, I keep forgetting about that :-[

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #21)
> (In reply to comment #20)
> >(In reply to comment #19)
> >>(From update of attachment 244821 [details] [diff] [review] [edit] [edit] [edit])
> >>>+   /** Used for select and select1 that are implemented by host document native
> >>>+    * widget using */
> >>Comment doesn't make sense.
> >Why?
> Well, it ends with the word "using" for a start.

It's possibly my english, sorry. How about this?

Used for select and select1 that are implemnted by host document's native widget 

Comment 23

[[e-mail]]

(In reply to comment #22)
>How about this?
>Used for select and select1 that are implemnted by host document's native widget
Now I see where the "using" comes in... you want "implemented using", I think.

Comment 24

[alexander :surkov (:asurkov)]

Attachment: patch3

Comment 25

[alexander :surkov (:asurkov)]

Attachment: redudrant label patch

(In reply to comment #16)

> 1) GetChildAtPoint is not working to get from the select into the option
> children

I guess it's the same problem like in bug 349644. When it will be fixed then problem will go away.

> 2) The checkboxes don't appear to be attached to their parent. I can't use MSAA
> Accessible Explorer to get to them.
> 3) The checkboxes don't appear to have an accessible name

selects full will have another implementation when bug 358712 will be checked in. Also these selects will have another implementation of accessible objects. I guess problem shouldn't exist for new implementation. 

> 4) I'm not sure that the extra groupbox is necessary for the compact
> presentation of the select. For the checkboxes it does make sense, but for the
> compact (list) appearance it seems like users will be confused if they're told
> their is a groupbox there. I know I told you to use ROLE_GROUPING, but do we
> need an accessible object for that item at all?

Now, compact selects (that are represented by listbox) have redundant accessible object in tree (html:label). This patch removes it. But probably you meant accessible object for xforms select control. If true then I'm sure that one is needed because at least it provides name/value/description accessibility properties.

Comment 26

[Aaron Leventhal]

Does that patch get rid of the extra ROLE_GROUPING? Or what does it fix?

Comment 27

[alexander :surkov (:asurkov)]

(In reply to comment #26)
> Does that patch get rid of the extra ROLE_GROUPING? Or what does it fix?
> 

It removes extra ROLE_LABEL.

Comment 28

[Aaron Leventhal]

Is the accessible name still correctly reported?

Comment 29

[aaronr]

Comment on attachment 247077 [details] [diff] [review]
patch3

>Index: accessible/public/nsIAccessibleProvider.idl
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/public/nsIAccessibleProvider.idl,v
>retrieving revision 1.10
>diff -u -8 -p -r1.10 nsIAccessibleProvider.idl
>--- accessible/public/nsIAccessibleProvider.idl	2 Nov 2006 14:50:32 -0000	1.10
>+++ accessible/public/nsIAccessibleProvider.idl	30 Nov 2006 16:55:30 -0000
>@@ -119,17 +119,19 @@ interface nsIAccessibleProvider : nsISup
>    /** Used for input[xsd:boolean] element */
>    const long XFormsInputBoolean = 0x00002005;
>    /** Used for secret element */
>    const long XFormsSecret = 0x00002006;
>    /** Used for range element represented by slider */
>    const long XFormsSliderRange = 0x00002007;
>    /** Used for xforms elements that provide accessible object for itself as
>     * well for anonymous content. This property are used for upload,
>-    * input[type="xsd:gDay"] and input[type="xsd:gMonth"].
>-    */
>+    * input[type="xsd:gDay"] and input[type="xsd:gMonth"] */
>    const long XFormsContainer = 0x00002008;
>+   /** Used for select and select1 that are implemented using host document's
>+    * native widget */
>+   const long XFormsSelect = 0x00002009;
> 

nit: Please put an example in the comment so that it is clear exactly what you mean.  Like, "For example, a select1 in a xhtml document may be represented by the native html control html:select"

>Index: accessible/src/xforms/nsXFormsFormControlsAccessible.h
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/xforms/nsXFormsFormControlsAccessible.h,v
>retrieving revision 1.4
>diff -u -8 -p -r1.4 nsXFormsFormControlsAccessible.h
>--- accessible/src/xforms/nsXFormsFormControlsAccessible.h	6 Nov 2006 02:50:37 -0000	1.4
>+++ accessible/src/xforms/nsXFormsFormControlsAccessible.h	30 Nov 2006 16:55:31 -0000
>@@ -144,10 +144,24 @@ public:
> 
>   // nsIAccessibleValue
>   NS_IMETHOD GetMaximumValue(double *aMaximumValue);
>   NS_IMETHOD GetMinimumValue(double *aMinimumValue);
>   NS_IMETHOD GetMinimumIncrement(double *aMinimumIncrement);
>   NS_IMETHOD GetCurrentValue(double *aCurrentValue);
> };
> 
>+
>+/**
>+ * Accessible object for xforms:select and xforms:select1 that are implemented
>+ * by host document native widget using.
>+ */
>+

nit: Looks like you missed correcting this comment per neil's suggestion.

With these nits fixed, r=me

Comment 30

[aaronr]

Comment on attachment 247185 [details] [diff] [review]
redudrant label patch

>Index: extensions/xforms/resources/content/select-xhtml.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/select-xhtml.xml,v
>retrieving revision 1.7
>diff -u -8 -p -r1.7 select-xhtml.xml
>--- extensions/xforms/resources/content/select-xhtml.xml	27 Oct 2006 10:49:09 -0000	1.7
>+++ extensions/xforms/resources/content/select-xhtml.xml	1 Dec 2006 16:47:19 -0000
>@@ -55,23 +55,21 @@
>   The section contains xforms select/select1 controls implementations for XHTML.
>   All controls are inherited from interface bindings realized in select.xml.
> -->
> 
>   <!-- SELECT APPEARANCE='COMPACT' : <DEFAULT> -->
>   <binding id="xformswidget-select-compact"
>            extends="chrome://xforms/content/select.xml#xformswidget-select-base">
>     <content>
>-      <html:label>
>-        <html:span class="label-container">
>-           <children includes="label"/>
>-        </html:span>
>-        <html:span anonid="control" xbl:inherits="style, accesskey"/>
>-        <children/>
>-      </html:label>
>+      <html:span class="label-container">
>+         <children includes="label"/>
>+      </html:span>
>+      <html:span anonid="control" xbl:inherits="style, accesskey"/>
>+      <children/>
>     </content>
>   </binding>

I don't have a problem with removing the html:label that contains these spans.  But was there an accessibility reason that you initially did it this way?  Does that reason no longer apply?

Comment 31

[alexander :surkov (:asurkov)]

Attachment: patch4 [for checkin]

Comment 32

[alexander :surkov (:asurkov)]

(In reply to comment #30)

> I don't have a problem with removing the html:label that contains these spans. 
> But was there an accessibility reason that you initially did it this way?  Does
> that reason no longer apply?
> 

Originally Doron did that in first version of select.xml. I can't see reason where it can be needed.

Comment 33

[Aaron Leventhal]

What's the answer to the question in comment 28?

Comment 34

[alexander :surkov (:asurkov)]

(In reply to comment #33)
> What's the answer to the question in comment 28?
> 

Sorry, I missed the question.

(In reply to comment #28)
> Is the accessible name still correctly reported?
> 

For sure, accessible name is formed from child xfroms:label element.

Comment 35

[Aaron Leventhal]

To Aaron Reed: that answer makes sense, as long as the layout and accessible name are correct, it should be good.

Comment 36

[alexander :surkov (:asurkov)]

Attachment: 247303: patch5 [for checkin]

Comment 37

[Håkan Waara]

Checked in for Alex.

Comment 38

[alexander :surkov (:asurkov)]

Waiting for review of 'redudrant label patch' patch.

Comment 39

[aaronr]

checked in redundant label patch to trunk for surkov.  This redundant label patch should go in the branches so that we keep the anonymous content consistent between all versions.  The accessibility patch should NOT go in the branches.

Comment 40

[aaronr]

for redundant label patch, checked into 1.8 branch on 2007-04-12 and checked into 1.8.0 branch on 2007-04-16