241991 - Active Accessibility: state change event at doc load finish shouldn't report STATE_BUSY

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

Steps to repro:
1. Run accessible event watcher from MSAA SDK (accevent.exe)
2. Watch for state change events only (under options)
3. Load a new page in Mozilla

What happens:
First state change event for page reports state_busy (this is correct, doc has
started to load)
Second state change event for page also reports state_busy (incorrect, doc has
finished loading)

Expected:
Second state change event should not report state_busy

Comment 1

[Aaron Leventhal]

Attachment: Make sure mBusy set correctly before firing event

Seeking r=kyle

Comment 2

[Kyle Yuan]

Comment on attachment 147252 [details] [diff] [review]
Make sure mBusy set correctly before firing event

sure, r=me.

Comment 3

[Aaron Leventhal]

Johnny, can we relax the requirement for sr='s in the accessibility module for
some patches?

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 147252 [details] [diff] [review]
Make sure mBusy set correctly before firing event


     if (mBusy != eBusyStateDone) {
 #ifndef MOZ_ACCESSIBILITY_ATK
+      mBusy = eBusyStateDone; // before event callback so STATE_BUSY is not
reported
       FireToolkitEvent(nsIAccessibleEvent::EVENT_STATE_CHANGE, this, nsnull);
 #endif
     }

Right after this code mBusy is set to eBusyStateDone too, why not simply move
that line up above the if check that this check is nested in?

If you feel that requiring sr slows down development or doesn't generally
improve code quality in the accessibility module, take that up with
[e-mail] and [e-mail].

sr=jst

Comment 5

[Aaron Leventhal]

> why not simply move that line up above the if check that this check is nested in?
Because the if () relies on the same value, so if we set it earlier it won't
fire the event.

> If you feel that requiring sr slows down development or doesn't generally
> improve code quality in the accessibility module, take that up with
> [e-mail] and [e-mail].
I did send an email to super-reviewers but got no response. 
Whenever I send an email to that alias I always get "your message awaits
moderator approval" and never hear back. Are the messages getting on there?

Comment 6

[Johnny Stenback (:jst)]

(In reply to comment #5)
> > If you feel that requiring sr slows down development or doesn't generally
> > improve code quality in the accessibility module, take that up with
> > [e-mail] and [e-mail].
> I did send an email to super-reviewers but got no response. 
> Whenever I send an email to that alias I always get "your message awaits
> moderator approval" and never hear back. Are the messages getting on there?

Hmm, I don't remember seeing your message, so it might have gotten lost. You'll
get the messages about awaiting moderator approval until someone approves
accepting email from your email address to the list, but you should only get
those a few times, then it should be taken care of. If you still get these,
please contact [e-mail]

Comment 7

[Aaron Leventhal]

This was checked in a while ago, forgot to mark it fixed.

2004-04-30 07:01	aaronleventhal%moonset.net 	mozilla/ accessible/ src/ base/
nsDocAccessible.cpp 	1.34 	1/0  	Bug 241991. State change event for doc should
clear busy flag when doc finished loading. r=kyle, sr=jst

Comment 8

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381