991969 - Event not fired when description changes

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

Str:
1. Open http://whatsock.com/test/dt_demo/Coding%20Arena/Inline%20Form%20Field%20Validation%20and%20Dynamic%20Help%20Tooltips/Dynamic%20Help%20Tooltip/demo.htm
2. Ensure focus is on the document itself.
3. Retrieve the accessible for the text input.
4. Observe that its description is empty.
5. Focus on the text input.
6. Wait a second.
Expected: A descriptionChange event should be fired on the text input indicating that its description has changed.
Actual: No event is fired.
7. Confirm that the description is no longer empty.
8. Type the letter f.
Expected: A descriptionChange event should be fired on the text input indicating that its description has changed.
Actual: No event is fired.
9. Confirm that the description has changed.

This test case uses aria-describedby, but the same applies if you change the title attribute. On Windows, EVENT_OBJECT_DESCRIPTIONCHANGE should be used.

I guess firing this all over the place might be a performance hit, so it might make sense to just fire when focused. That said, we may find test cases which need it for non-focused elements too; I'm not sure.

Related NVDA issue ticket: http://community.nvda-project.org/ticket/4047

Comment 1

[alexander :surkov (:asurkov)]

How is a description provided?

Comment 2

[James Teh [:Jamie]]

Not sure I understand the question. In terms of the DOM, aria-describedby or title. In terms of MSAA exposure, IAccessible::accDescription.

Comment 3

[Marco Zehe (:MarcoZ)]

(In reply to James Teh [:Jamie] from comment #0)
> I guess firing this all over the place might be a performance hit, so it
> might make sense to just fire when focused. That said, we may find test
> cases which need it for non-focused elements too; I'm not sure.

I think the more forward-thinking thing would be to always fire it. I don't think adjusted titles or aria-describedby constructs are that common yet, so we won't have hundreds of events to fire on a page I think.

Related question: Does NVDA do anything with accDescription in its virtual buffers at all, or only when speaking a focused item?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #2)
> Not sure I understand the question. In terms of the DOM, aria-describedby or
> title.

I'm about DOM of course. There's a number of ways to change accessible description and I wouldn't want to support each of tehm because of perfromance. For example, you can do

<div aria-describedby="aga"></div>
<p id="aga></p>

p.textContent = "description has been changed";

or more crazy examples. I wouldn't support these until it's really necessary. As I understand you have a specific use case and what I would do here is I would fix this example only leaving other possible use case out of the scope of this bug.

Comment 5

[James Teh [:Jamie]]

(In reply to Marco Zehe (:MarcoZ) from comment #3)
> Related question: Does NVDA do anything with accDescription in its virtual
> buffers at all, or only when speaking a focused item?
The virtual buffer does render it, but only for use with quick navigation or focus.

(In reply to alexander :surkov from comment #4)
> I'm about DOM of course. There's a number of ways to change accessible
> description and I wouldn't want to support each of tehm because of
> perfromance.
Note that Firefox already does something similar for nameChange (aria-label, aria-labelledby, alt and countless other ways). Why is description less important?

> As I understand you have a specific use case and what I would do
> here is I would fix this example only leaving other possible use case out of
> the scope of this bug.
Note that i didn't come up with this example, so I have no idea what the author intends to do beyond this specific test case.

Comment 6

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 7

[Jonathan Wei [:jwei]]

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

Review of attachment 8405487 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/generic/DocAccessible.cpp
@@ +972,5 @@
>  
> +  // Fire name change and description change events. XXX: it's not complete and
> +  // dupes the code logic of accessible name and description calculation, we do
> +  // that for performance reasons.
> +  if (aAttribute == nsGkAtoms::aria_label) {

A bit annoying that the cases need to be handled this way, but at least the comment calls it out.

::: accessible/tests/mochitest/events/test_descrchange.html
@@ +30,5 @@
> +      this.invoke = function setAttr_invoke()
> +      {
> +        getNode(aID).setAttribute(aAttr, aValue);
> +      }
> +      

Nit: remove whitespace.

::: accessible/tests/mochitest/events/test_namechange.html
@@ +30,5 @@
> +      this.invoke = function setAttr_invoke()
> +      {
> +        getNode(aID).setAttribute(aAttr, aValue);
> +      }
> +      

Nit: remove whitespace.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to Jonathan Wei [:jwei] from comment #7)

> > +  // Fire name change and description change events. XXX: it's not complete and
> > +  // dupes the code logic of accessible name and description calculation, we do
> > +  // that for performance reasons.
> > +  if (aAttribute == nsGkAtoms::aria_label) {
> 
> A bit annoying that the cases need to be handled this way, but at least the
> comment calls it out.

right, just didn't want to rework the code, I hope we will do that one day

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6e7ef50feb0f