When an ARIA state liked checked or expanded changes, we should fire the correct ATK state change event.

Test cases: http://www.mozilla.org/access/dhtml/checkbox (checked) http://www.mozilla.org/access/dhtml/tree (expanded)

Ginn, I didn't test the patch but it should work. Will you try it for checkbox and tree?

Comment on attachment 259167 [details] [diff] [review] Set the stateData that goes with the state change event aaron, this doesn't work because stateData will be gone after the timer. We cannot use &stateData in this case.

Yes that was very stupid of me! It reminded me of all the things we need to do for the event system in general, so I filed bug 374790 on that. For this bug perhaps the answer is to pass the actual struct around instead of a pointer to it. Unfortunately each kind of event data is a different struct. So we probably need to turn it into a class and have each kind of event data be a different class, which inherits from a base event data class. Does that make sense?

Alexander, if you get a chance to look at this and bug 374790 it would be great.

(In reply to comment #5) > Alexander, if you get a chance to look at this and bug 374790 it would be > great. > Yes, I will. I'll try to do soon.

Surkov, when you get to this, can you fix bug 255088 at the same time? I think the best thing is just to fire the STATE_CHANGE with event data for STATE_SENSITIVE, when the disabled attribute changes no matter what namespace the attribute is in. That will catch XUL, ARIA and HTML disabled state changes.

Comment on attachment 261669 [details] [diff] [review] patch2 I have a feeling that new event constructor is too early to detect if state is set, because attribute change hasn't occured yet. I think it was why we used FireDelayedToolkitEvent() instead of FireToolkitEvent() (which we should comment about). Test it, because I might be wrong. You can use http://www.mozilla.org/access/dhtml/checkbox with with accevent from MSAA SDK to test.

Comment on attachment 261669 [details] [diff] [review] patch2 Actually I tested on Linux and this works perfectly, so ignore my previous comment. But, test and make sure that events on Windows still work. mIsEnabled = (mIsExtraState ? extraState : state) & mState; This needs to be mIsEnabled = 0 != ((mIsExtraState ? extraState : state) & mState); Or !!(mIsExtraState ? extraState : state) & mState) or saomething like that, otherwise the boolean gets an int. Optimization for ARIAAttributeChanged(), pull this out to the top of the method, before the block of if's: + nsCOMPtr<nsIAccessibleEvent> event; In each attribute check, just set the event, don't fire or return. Then at the end of the method, do: if (event) { FireDelayedAccessibleEvent(event); } For disabled, we also need to fire state change for sensitive. Also, we need to do that one for HTML and XUL namespace as well. See bug 255088

Comment on attachment 261669 [details] [diff] [review] patch2 Actually you can have + if you deal with my comments in the checkin or in another bug.

The patch has one problem related with delayed events. Sometimes accessible is shutdowned before delayed event is proccessed. I'll file new bug for this.

checked in

It doesn't look like the 2nd part of comment 10 was checked in, right?

(In reply to comment #14) > It doesn't look like the 2nd part of comment 10 was checked in, right? > Do you mean this? (In reply to comment #10) > In each attribute check, just set the event, don't fire or return. > Then at the end of the method, do: > if (event) { > FireDelayedAccessibleEvent(event); > } Yes, I missed. Sorry. Should I fix it? (I'm just not big fan of if/else statements).