Last Comment Bug 659018 - missing property-changed::accessible-name on inbox refresh in Thunderbird
: missing property-changed::accessible-name on inbox refresh in Thunderbird
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: mozilla7
Assigned To: Trevor Saunders (:tbsaunde)
:
Mentors:
: 493685 (view as bug list)
Depends on: 666337
Blocks: eventa11y
  Show dependency treegraph
 
Reported: 2011-05-23 09:45 PDT by Mike Gorse
Modified: 2011-09-03 17:49 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.34 KB, patch)
2011-06-15 22:41 PDT, Trevor Saunders (:tbsaunde)
surkov.alexander: review+
Details | Diff | Splinter Review

Description Mike Gorse 2011-05-23 09:45:03 PDT
User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0.0) Gecko/20100101 Firefox/4.0
Build Identifier: Thunderbird 3.4a1pre

When the inbox is refreshed in Thunderbird and the number of unread messages changes, the inbox label (and consequently the name of its accessible) changes, but a property-changed::accessible-name is not fired.  This causes problems with Orca and AT-SPI2 since AT-SPI expects to receive the event and continues to read the old cached value (unless AT-SPI 2.0.2 or later is used and setCacheMask is called to disable caching of the name--I plan on proposing that Orca do this for now as a work-around).

As I was testing, I saw the property-changed event being fired when I had the setCacheMask work-around in place but not when I did not have it included, so either this only happens sometimes or the event can be triggered somehow (perhaps calling atk_object_get_name on the accessible triggers the event).  I'm marking this as "happens sometimes" since I'm not entirely sure which is the case.

Reproducible: Sometimes

Steps to Reproduce:
1. Start Orca.  If there is a setCacheMask call in the Thunderbird script, then remove it.
2. Open Thunderbird.
3. Do something to cause the number of unread inbox messages to change.

Actual Results:  
Orca continues to read the old inbox label.

Expected Results:  
I would have expected Thunderbird to send a property-changed::accessible-name when the label gets updated, so Orca would read the new label when it gains focus.  Note that it should also be possible to reproduce this with Accerciser using similar steps.
Comment 1 Marco Zehe (:MarcoZ) on PTO until August 15 2011-05-23 10:24:33 PDT
FWIW, NVDA always picks up the change in unread messages on Windows.
Comment 2 Fernando Herrera 2011-05-23 11:25:28 PDT
I think we are not handling nsIAccessibleEvent::EVENT_NAME_CHANGE on the atk code.
Comment 3 alexander :surkov 2011-05-23 20:43:10 PDT
(In reply to comment #2)
> I think we are not handling nsIAccessibleEvent::EVENT_NAME_CHANGE on the atk
> code.

correct, we could use this event to update atk name (atk_object_set_name), I think this should emit this event automatically.

Fernando, Trevor, can you take it?
Comment 4 Trevor Saunders (:tbsaunde) 2011-06-15 22:41:46 PDT
Created attachment 539725 [details] [diff] [review]
patch

atk_set_name() sends the proper event for us, so on a name change event we can just see if the name has actually changed and if so set it causing atk to send the signal for us.
Comment 5 alexander :surkov 2011-06-16 00:26:46 PDT
Comment on attachment 539725 [details] [diff] [review]
patch

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

::: accessible/src/atk/nsAccessibleWrap.cpp
@@ +1082,5 @@
> +        nsString newName;
> +        accessible->GetName(newName);
> +        NS_ConvertUTF16toUTF8 utf8Name(newName);
> +        if (!utf8Name.Equals(atkObj->name))
> +          atk_object_set_name(atkObj, utf8Name.get());

does it really make sense to check whether name weren't changed? I'd guess this can happen iff atk name was requested before we handled the event. Do you handle this case?
Comment 6 Trevor Saunders (:tbsaunde) 2011-06-16 02:56:46 PDT
(In reply to comment #5)
> Comment on attachment 539725 [details] [diff] [review] [review]
> patch
> 
> Review of attachment 539725 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/atk/nsAccessibleWrap.cpp
> @@ +1082,5 @@
> > +        nsString newName;
> > +        accessible->GetName(newName);
> > +        NS_ConvertUTF16toUTF8 utf8Name(newName);
> > +        if (!utf8Name.Equals(atkObj->name))
> > +          atk_object_set_name(atkObj, utf8Name.get());
> 
> does it really make sense to check whether name weren't changed? I'd guess
> this can happen iff atk name was requested before we handled the event. Do
> you handle this case?

I think the cost is pretty low, and it ensures that we don't have a race like you describe where we send an event say the name has changed when it actually hasn't.  I'm not sure what you mean by the do you handle this case question.  If your asking why I make the check then I think, if something else not sure.
Comment 7 alexander :surkov 2011-06-20 22:03:56 PDT
Comment on attachment 539725 [details] [diff] [review]
patch

ok, r=me
Comment 8 Trevor Saunders (:tbsaunde) 2011-06-21 21:28:14 PDT
http://hg.mozilla.org/mozilla-central/rev/b7a93f1279b7
Comment 9 Trevor Saunders (:tbsaunde) 2011-09-03 17:49:52 PDT
*** Bug 493685 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.