558036 - make HTML <output> accessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

arbitrary HTML or @value attribute may define content of HTML output element. Also HTML output has @for attribute pointing to dependent elements - "The for  content attribute allows an explicit relationship to be made between the result of a calculation and the elements that represent the values that went into the calculation or that otherwise influenced the calculation." It looks like RELATION_CONTROLLED_BY/RELATION_CONTROLLER_FOR accessible relations are suitable for mapping of @for attribute.

Comment 1

[alexander :surkov (:asurkov)]

(In reply to bug 346485 comment #14)
> (In reply to bug 346485 comment #11)
> > I have couple question to understand what we need to do for a11y here.
> > 
> > 1) What frame is used for HTML output element?
> 
> The output element has no specific frame.

It's necessary to know what frame class is used for it because frames creates accessible objects. Since I suppose HTML output might require to introduce special accessibility class for it then we need to change GetAccessible() method of its frame.

> 
> > 2) Can HTML output contain arbitrary HTML or its content is always defined by
> > @value attribute?
> 
> The output element can contain arbitrary HTML. But it's @value will only
> reflect the text from this html.

If author provides @value only and no HTML content then what kind of content does output have?

> 
> You can consider the output element like a span element. It has to be used for
> web developers to show something depending on other fields value and it adds
> the ability to check the element value validity. Even if the constraint
> validity API is not landed yet.

Can you point spec for validity API? We might need to address this in a11y somehow (or not).

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #0)
> arbitrary HTML or @value attribute may define content of HTML output element.
> Also HTML output has @for attribute pointing to dependent elements - "The for 
> content attribute allows an explicit relationship to be made between the result
> of a calculation and the elements that represent the values that went into the
> calculation or that otherwise influenced the calculation." It looks like
> RELATION_CONTROLLED_BY/RELATION_CONTROLLER_FOR accessible relations are
> suitable for mapping of @for attribute.

Yes exactly my thinking too. (Thank you IA2)

Comment 3

[Mounir Lamouri (:mounir)]

(In reply to comment #1)
> (In reply to bug 346485 comment #14)
> > (In reply to bug 346485 comment #11)
> > > I have couple question to understand what we need to do for a11y here.
> > > 
> > > 1) What frame is used for HTML output element?
> > 
> > The output element has no specific frame.
> 
> It's necessary to know what frame class is used for it because frames creates
> accessible objects. Since I suppose HTML output might require to introduce
> special accessibility class for it then we need to change GetAccessible()
> method of its frame.

Unfortunately, I don't think I can help here.

> > > 2) Can HTML output contain arbitrary HTML or its content is always defined by
> > > @value attribute?
> > 
> > The output element can contain arbitrary HTML. But it's @value will only
> > reflect the text from this html.
> 
> If author provides @value only and no HTML content then what kind of content
> does output have?

To summarize, the value IDL attribute acts like the element textContent IDL. So, when the value is set, the textContent will be set too. The textContent could also containt the defaultValue if there is no value.

> > You can consider the output element like a span element. It has to be used for
> > web developers to show something depending on other fields value and it adds
> > the ability to check the element value validity. Even if the constraint
> > validity API is not landed yet.
> 
> Can you point spec for validity API? We might need to address this in a11y
> somehow (or not).

bug 345624 is about the constraint validation API. David is already in the CC list of this bug.

Comment 4

[Henri Sivonen (:hsivonen) (not reading bugmail while on leave)]

(In reply to comment #0)
> arbitrary HTML or @value attribute may define content of HTML output element.

The content of the output element is always simply the descendant nodes of the element. There is no @value. There is only a .value DOM property, but accessibility doesn't need to care about it, because it's only syntactic sugar for JavaScript.

> Also HTML output has @for attribute pointing to dependent elements - "The for 
> content attribute allows an explicit relationship to be made between the result
> of a calculation and the elements that represent the values that went into the
> calculation or that otherwise influenced the calculation." It looks like
> RELATION_CONTROLLED_BY/RELATION_CONTROLLER_FOR accessible relations are
> suitable for mapping of @for attribute.

That seems to be the main accessibility difference from <span>.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: WIP (for aria live stuff)

Hanging a patch. Folks want the live attribute for output. The test currently fails -- unable to get accessible. Probably we're doing our text catch-all.

Comment 7

[Marco Zehe (:MarcoZ)]

Comment on attachment 471918 [details] [diff] [review]
WIP (for aria live stuff)

We're currently not creating any accessible for the html:output element. We simply create a text leaf. So we need logic in nsAccessibilityService to create a section accessible or similar for html:output.

Comment 8

[alexander :surkov (:asurkov)]

Either we should change nsIFrame::CreateAccessible on frame used for HTML output or change nsAccessibiltiyService::CreateAccessbileFromMarkup to handle this. If HTML output implementation is entirely inside of content module then the second approach might be preferable (at least we'll create output accessible for display block and inline when two different frame classes are used).

Comment 9

[Mounir Lamouri (:mounir)]

(In reply to comment #8)
> If HTML output implementation is entirely inside of content module

It is (except the parser part of course).

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

OK I can add the case to nsAccessibilityService when I find a slot of time.

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch

This patch:
1. Creates an accessible for output.
2. Provides a default live attribute (polite).
3. Adds a controlled by relation indicating the 'for' attribute ids.
4. Tests 2 and 3.

I think this might be enough for our initial support. I'm a little unsure about the form attribute. We could add it to the controlled by relations but it doesn't seem to fit. It might be worth leaving this for a follow up after some discussion with AT devs.

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 476172 [details] [diff] [review]
patch

I'm not sure why you ask for feedback but in general the patch looks correct, f+, though that's not a review

Comment 13

[Marco Zehe (:MarcoZ)]

Comment on attachment 476172 [details] [diff] [review]
patch

What made you choose ROLE_SECTION for html:output? Does it have the typical layout properties associated with a div, or is it actually more like a span element, in which case we should expose it with the same role as html:span?

With that answered/changed, f=me.

Comment 14

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #12)
> Comment on attachment 476172 [details] [diff] [review]
> patch
> 
> I'm not sure why you ask for feedback but in general the patch looks correct,
> f+, though that's not a review

Do you think the controlled by relation based on the 'for' attribute is enough?

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #13)
> Comment on attachment 476172 [details] [diff] [review]
> patch
> 
> What made you choose ROLE_SECTION for html:output? Does it have the typical
> layout properties associated with a div, or is it actually more like a span
> element, in which case we should expose it with the same role as html:span?

Good question, and there might be a better role but I haven't thought of it. We don't have a role mapping for html:span. On windows we'll map ROLE_SECTION via MSAA to BSTR "output" and IA2 to IA2_ROLE_SECTION.

Comment 16

[David Bolter [:davidb] (NeedInfo me for attention)]

Oh and of course for Linux: ATK_ROLE_SECTION (since IA2 is based on atk I tend to assume parity).

Comment 17

[alexander :surkov (:asurkov)]

(In reply to comment #14)
> (In reply to comment #12)
> > Comment on attachment 476172 [details] [diff] [review] [details]
> > patch
> > 
> > I'm not sure why you ask for feedback but in general the patch looks correct,
> > f+, though that's not a review
> 
> Do you think the controlled by relation based on the 'for' attribute is enough?

actually we should expose controller for relation as well, that's might be tricky.

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 1.1 (includes controller for relation)

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 1.2 (adds a11y tree test)

Comment 20

[David Bolter [:davidb] (NeedInfo me for attention)]

Note I fixed the indenting for test_formctrl locally.

Comment 21

[Marco Zehe (:MarcoZ)]

Comment on attachment 476282 [details] [diff] [review]
patch 1.2 (adds a11y tree test)

r=me, thanks!

Comment 22

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 1.3

Cleanup (including an addref - oops!)

Comment 23

[alexander :surkov (:asurkov)]

Comment on attachment 476301 [details] [diff] [review]
patch 1.3

r=me
requesting for approval, we need to have HTML 5 controls accessible

Comment 24

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch to land

Comment 25

[alexander :surkov (:asurkov)]

Comment on attachment 476309 [details] [diff] [review]
patch to land

looks ok, few nits you might want to fix

> 
>+  if (tag == nsAccessibilityAtoms::output) {
>+    nsAccessible* accessible = new nsHTMLOutputAccessible(aContent, aWeakShell);
>+
>+    NS_IF_ADDREF(accessible);

no new line between them

>   case nsIAccessibleRelation::RELATION_CONTROLLER_FOR:
>     {
>-      return nsRelUtils::
>+      nsresult rv = nsRelUtils::
>         AddTargetFromIDRefsAttr(aRelationType, aRelation, mContent,
>                                 nsAccessibilityAtoms::aria_controls);
>+
>+      NS_ENSURE_SUCCESS(rv,rv);

it's not really necessary to have new line here as well

>+NS_IMETHODIMP
>+nsHTMLOutputAccessible::GetRelationByType(PRUint32 aRelationType,
>+                                          nsIAccessibleRelation** aRelation)
>+{
>+  nsresult rv = nsAccessibleWrap::GetRelationByType(aRelationType, aRelation);
>+
>+  NS_ENSURE_SUCCESS(rv, rv);

the same

Comment 26

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 476309 [details] [diff] [review]
patch to land

Approved for 2.0 landing.

Comment 27

[David Bolter [:davidb] (NeedInfo me for attention)]

Landed: http://hg.mozilla.org/mozilla-central/rev/d95e7b7cb336

Thanks all.