xpcom_core.dll!NS_DebugBreak_P(unsigned int aSeverity=0x00000001, const char * aStr=0x015dc2e0, const char * aExpr=0x015dc2f8, const char * aFile=0x015dc12c, int aLine=0x00000209) Line 354 + 0xc bytes C++ accessibility.dll!nsCOMPtr<nsIAccessibleEvent>::Assert_NoQueryNeeded() Line 521 + 0x2b bytes C++ accessibility.dll!nsCOMPtr<nsIAccessibleEvent>::nsCOMPtr<nsIAccessibleEvent>(nsIAccessibleEvent * aRawPtr=0x0664f16c) Line 557 C++ accessibility.dll!nsTArrayElementTraits<nsCOMPtr<nsIAccessibleEvent> >::Construct<nsIAccessibleEvent *>(nsCOMPtr<nsIAccessibleEvent> * e=0x06657670, nsIAccessibleEvent * const & arg=0x0664f16c) Line 193 + 0x25 bytes C++ accessibility.dll!nsTArray<nsCOMPtr<nsIAccessibleEvent> >::AssignRange<nsIAccessibleEvent *>(unsigned int start=0x00000000, unsigned int count=0x00000001, nsIAccessibleEvent * const * values=0x0012d094) Line 875 + 0xd bytes C++ accessibility.dll!nsTArray<nsCOMPtr<nsIAccessibleEvent> >::AppendElements<nsIAccessibleEvent *>(nsIAccessibleEvent * const * array=0x0012d094, unsigned int arrayLen=0x00000001) Line 619 C++ accessibility.dll!nsTArray<nsCOMPtr<nsIAccessibleEvent> >::AppendElement<nsIAccessibleEvent *>(nsIAccessibleEvent * const & item=0x0664f16c) Line 633 C++ accessibility.dll!nsDocAccessible::FireDelayedAccessibleEvent(nsIAccessibleEvent * aEvent=0x0664f16c) Line 1623 C++ > accessibility.dll!nsDocAccessible::AttributeChangedImpl(nsIContent * aContent=0x02727768, int aNameSpaceID=0x00000000, nsIAtom * aAttribute=0x00d5cf78) Line 1165 C++

I tried to understand why QueryInterface returns different object than original one in the case of nsAccStateChangeEvent but QI implementation looks like a bit complex. The fix would be more nice if we could change nsAccStateChangeEvent so that QI will return the same object but I have no idea how. Neil, if you have some ideas please share with us. Thank you!

I see another similar assertion: xpcom_core.dll!Break(const char * aMsg=0x0012e2c0) Line 489 C++ xpcom_core.dll!NS_DebugBreak_P(unsigned int aSeverity=0x00000001, const char * aStr=0x012ec2e0, const char * aExpr=0x012ec2f8, const char * aFile=0x012ec12c, int aLine=0x00000209) Line 354 + 0xc bytes C++ accessibility.dll!nsCOMPtr<nsIAccessibleEvent>::Assert_NoQueryNeeded() Line 521 + 0x2b bytes C++ accessibility.dll!nsCOMPtr<nsIAccessibleEvent>::nsCOMPtr<nsIAccessibleEvent>(nsIAccessibleEvent * aRawPtr=0x0502fddc) Line 557 C++ accessibility.dll!nsTArrayElementTraits<nsCOMPtr<nsIAccessibleEvent> >::Construct<nsIAccessibleEvent *>(nsCOMPtr<nsIAccessibleEvent> * e=0x0508cecc, nsIAccessibleEvent * const & arg=0x0502fddc) Line 193 + 0x25 bytes C++ accessibility.dll!nsTArray<nsCOMPtr<nsIAccessibleEvent> >::AssignRange<nsIAccessibleEvent *>(unsigned int start=0x00000001, unsigned int count=0x00000001, nsIAccessibleEvent * const * values=0x0012e764) Line 875 + 0xd bytes C++ accessibility.dll!nsTArray<nsCOMPtr<nsIAccessibleEvent> >::AppendElements<nsIAccessibleEvent *>(nsIAccessibleEvent * const * array=0x0012e764, unsigned int arrayLen=0x00000001) Line 619 C++ accessibility.dll!nsTArray<nsCOMPtr<nsIAccessibleEvent> >::AppendElement<nsIAccessibleEvent *>(nsIAccessibleEvent * const & item=0x0502fddc) Line 633 C++ accessibility.dll!nsDocAccessible::FireDelayedAccessibleEvent(nsIAccessibleEvent * aEvent=0x0502fddc) Line 1631 C++ > accessibility.dll!nsCaretAccessible::NormalSelectionChanged(nsIDOMDocument * aDoc=0x0509bc4c, nsISelection * aSel=0x05096058) Line 276 + 0x14 bytes C++ accessibility.dll!nsCaretAccessible::NotifySelectionChanged(nsIDOMDocument * aDoc=0x0509bc4c, nsISelection * aSel=0x05096058, short aReason=0x0003) Line 228 + 0x10 bytes C++ I think we should get it whenever we pass event object derived from nsAccEvent to FireDelayedEvents. So it should fixed also. I think I'll wait for Neil and David comments before to put next patch.

(In reply to comment #1) > I tried to understand why QueryInterface returns different object than original > one in the case of nsAccStateChangeEvent but QI implementation looks like a bit > complex. The fix would be more nice if we could change nsAccStateChangeEvent so > that QI will return the same object but I have no idea how. OK, so this is a problem with multiple inheritance. nsAccStateChangeEvent derives from nsIAccessibleStateChangeEvent which derives from nsIAccessibleEvent, but nsAccStateChangeEvent additionally derives from nsAccEvent which also derives from nsIAccessibleEvent. Thus the cast from nsAccStateChangeEvent to nsIAccessibleEvent is ambiguous. Since you have used NS_IMPL_ISUPPORTS_INHERITED, the QueryInterface call uses the nsAccStateChangeEvent -> nsAccEvent -> nsIAccessibleEvent cast, but your event creation code uses the nsAccStateChangeEvent -> nsIAccessibleStateChangeEvent -> nsIAccessibleEvent cast. This nsIAccessibleEvent is the "wrong" pointer (you can still call all the methods and it will work, but nsCOMPtr doesn't like it.) Unlike nsCOMPtr, nsCOMArray doesn't care, but this does explain why you had to use do_QueryInterface when retriving your nsIAccessibleEvent from the array.

There are a number of options you can take: 1. Stop inheriting nsIAccessibleStateChangeEvent from nsIAccessibleEvent. This will make it harder to call nsIAccessibleEvent methods on an nsIAccessibleStateChangeEvent object because you'll have to do_QueryInterface each time. On the other hand, if you generally work the other way around (i.e. you get given an nsIAccessibleEvent, and need to QueryInterface to an nsIAccessibleStateChangeEvent every time), then this isn't a problem. 2. Stop inheriting nsAccStateChangeEvent from nsAccEvent. Instead, create a new class nsAccEvent_base which contains the methods but doesn't claim to implement nsIAccessibleEvent. (This is probably infeasible in your case.) 3. Be more careful when casting an nsAccStateChangeEvent to nsIAccessibleEvent.

Neil, thank you for detailed answer. I though about 1th option but it will force me to query nsIAccessibleEvent each time even if I don't need to pass it to methods taking nsIAccessibleEvent* argument. 3d option is what I don't like a lot (this is approach of my patch I think). 2nd option to aggregate some base class might be good since all derivatives from nsAccEvent redefines nsIAccessibleEvent by NS_FORWARD. However there is always multiple inheritance from nsISupports which doesn't lead us to unambiguity if I get things right. Could I change somehow nsISupports::QueryInterface implementation to avoid this problem? Could you give details about object creation for classes using multiple inheritance? How are they allocated in memory and how many objects are created for single instance? For example, if I have class nsISupports {}; class nsIFoo : public nsISupports {}; class nsIFoo2 : public nsISupports {}; class nsFoo : public nsIFoo {}; class nsFoo : public nsIFoo, public nsIFoo2 {}; then how much objects will be created when I create nsFoo and nsFoo2 instances?

I'm not sure I understood your question, so I'll use nsAcc*: Current layout: nsISupports: vtable ptr nsIAccessibleEvent: vtable ptr (first 3 entries shared with nsISupports) nsIAccessibleStateChangeEvent: vtable ptr (initial entries shared) nsAccEvent: vtable ptr (shared with nsIAccessibleEvent/nsISupports) nsAccEvent members nsAccStateChangeEvent: vtable ptr (shared with nsAccEvent etc.) nsAccEvent members vtable ptr (for nsIAccessibleStateChangeEvent) nsAccStateChangeEvent members The first vtable for nsAccStateChangeEvent contains: # nsISupports vtable entries # nsIAccessibleEvent vtable entries # Any new virtual methods defined on nsAccEvent # Any new virtual methods defined on nsAccStateChangeEvent The second vtable for nsAccStateChangeEvent contains: # nsISupports vtable entry stubs (casts to the "main" nsISupports) # nsIAccessibleEvent vtable entry stubs* # nsIAccessibleStateChangeEvent entries *Breaking the nsIAccessibleStateChangeEvent -> nsIAccessibleEvent inheritance would remove these entries from the vtable. Layout with nsAccEvent_base: nsISupports: vtable ptr nsIAccessibleEvent: vtable ptr (first 3 entries shared with nsISupports) nsIAccessibleStateChangeEvent: vtable ptr (initial entries shared) nsAccEvent_base: vtable ptr (if any virtual methods are defined) nsAccEvent_base members nsAccStateChangeEvent: vtable ptr (shared with nsIAccessibleStateChangeEvent) nsAccEvent vtable ptr (if applicable) nsAccEvent members nsAccStateChangeEvent members The vtable for nsAccStateChangeEvent contains: # nsISupports vtable entries # nsIAccessibleEvent vtable entries # nsIAccessibleStateChangeEvent vtable entries # Any new virtual methods defined on nsAccStateChangeEvent An nsISupports* can point to any vtable ptr that is shared with nsISupports but an nsCOMPtr<nsISupports> can only point to the main nsISupports.

I tend to think the easiest and I guess performant way is to stop inheritance from nsIAccessibleEvent. nsAccEventBase_ approach makes me more happy but it makes implementation more complicated. What do you say?

Comment on attachment 400722 [details] [diff] [review] patch2 Wow, this way seems to have turned out to be quite easy! >+ nsCOMPtr<nsIAccessibleEvent> event(do_QueryInterface(gTextEvent)); >+ > nsCOMPtr<nsIAccessible> targetAcc; >- gTextEvent->GetAccessible(getter_AddRefs(targetAcc)); >+ event->GetAccessible(getter_AddRefs(targetAcc)); Closed system, so the QueryInterface will never fail, right?

(In reply to comment #8) > > nsCOMPtr<nsIAccessible> targetAcc; > >- gTextEvent->GetAccessible(getter_AddRefs(targetAcc)); > >+ event->GetAccessible(getter_AddRefs(targetAcc)); > Closed system, so the QueryInterface will never fail, right? It shouldn't :) At least nsIAccessibleTableChangeEvent is implemented by nsAccTextChangeEvent only which is inherited from nsAccEvent.

I meant nsIAccessibleTextChangeEvent instead of table change event

Comment on attachment 400722 [details] [diff] [review] patch2 Wow. r=me. I'm so glad you banged the ambiguous inheritance problem out with neil before I got here :) >+ // Universal boolean properties that don't require a role. Fire the state >+ // change whether disabled or aria-disabled attribute is set. nit: change "whether" to "when"

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/7183bd1853be (David's nit is addressed)