Event not fired when description changes

RESOLVED FIXED in mozilla31

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jamie, Assigned: surkov)

Tracking

(Blocks: 1 bug)

Trunk
mozilla31
x86_64
Windows 8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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
(Assignee)

Comment 1

5 years ago
How is a description provided?
Blocks: 472809
(Reporter)

Comment 2

5 years ago
Not sure I understand the question. In terms of the DOM, aria-describedby or title. In terms of MSAA exposure, IAccessible::accDescription.
(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?
(Assignee)

Comment 4

5 years ago
(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.
(Reporter)

Comment 5

5 years ago
(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.
(Assignee)

Comment 6

5 years ago
Created attachment 8405487 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Attachment #8405487 - Flags: review?(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.
Attachment #8405487 - Flags: review?(jwei) → review+
(Assignee)

Comment 8

5 years ago
(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
https://hg.mozilla.org/mozilla-central/rev/6e7ef50feb0f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.