232854 - XULDocument.RemoveBroadcastListenerFor does not remove the observer

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Wang Xianzhu]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv:1.4) Gecko/20030624

Whenver XULDocument.RemoveBroadcastListenerFor is called with correct arguments,
it will return NS_OK, but the observer is not removed.

964         for (PRInt32 i = entry->mListeners.Count() - 1; i >= 0; --i) {
965             BroadcastListener* bl =
966                 NS_STATIC_CAST(BroadcastListener*, entry->mListeners[i]);
967 
968             if ((bl->mListener == aListener) && (bl->mAttribute == attr)) {
969                 entry->mListeners.RemoveElement(aListener);
970 
971                 if (entry->mListeners.Count() == 0)
972                     PL_DHashTableOperate(mBroadcasterMap, aBroadcaster,
973                                          PL_DHASH_REMOVE);
974 
975                 SynchronizeBroadcastListener(aBroadcaster, aListener, aAttr);
976 
977                 break;
978             }
979         }

The problem is caused by line 969, because the entries in entry->mListeners are
of type BroadcastListener.  '...RemoveElement(aListener)' does nothing.

This may cause crash if the listener element is removed from the document, and
GC'ed by Javascript.


Reproducible: Always
Steps to Reproduce:

Comment 1

[Chase Tingley]

Yup, this should be |RemoveElement(bl)|

-> XUL

Comment 2

[Boris Zbarsky [:bzbarsky]]

Post a patch?  hyatt is not likely to....

Comment 3

[Chase Tingley]

Attachment: patch

Actually, RemoveElement(i) would be even better...

Comment 4

[Jan Varga [:janv]]

Comment on attachment 140475 [details] [diff] [review]
patch

r=varga

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 140475 [details] [diff] [review]
patch

sr=bzbarsky

Comment 6

[Chase Tingley]

Fix checked in.