Created attachment 387021 [details]
Testcase (uses enhanced privileges)
Not likely to be a common scenario, I'll be the first to admit, but:
1. Multiple objects register themselves as a document state listener on an editor
2. During a callback, one of them decides it's no longer interested in notifications, and removes itself as a listener during the callback
3. Crash with "pure virtual method called
terminate called without an active exception
Created attachment 387045 [details] [diff] [review]
So, nsEditor::NotifyDocumentListeners does 2 bad things:
1) Assumes a listener won't remove itself during the callback (bad, as it's not prohibited by the API, but not the cause of the crash)
2) When looping over the array of listeners, it reads the size of the array once, and assumes it never changes. This is what gets us in to trouble, of course.
The patch tackles both. Just using mDocStateListeners.Count() in the loop condition wouldn't be sufficient, as it would avoid the crash, but potentially cause a listener to be skipped when sending out notifications (if the previous listener in the array removed itself).
(Incidentally, the notifications for nsIEditActionListeners have that problem).
I can't see an easy way to test for the NotifyDocumentCreated notification at the moment.
Comment on attachment 387045 [details] [diff] [review]
1. i don't think peterv is an editor peer, so please don't ask him to review.
you can ask glazou, neil, brade, smfr, or myself for reviews
>- rv = mDocStateListeners[i]->NotifyDocumentCreated();
>+ rv =listeners[i]->NotifyDocumentCreated();
please don't accidentally lose spaces after = :/
(In reply to comment #2)
> (From update of attachment 387045 [details] [diff] [review])
> 1. i don't think peterv is an editor peer, so please don't ask him to review.
peterv did the contentEditable implementation, and has reviewed every editor patch I've done over the past two years. He's been the most active (the only?) reviewer in editor for a while. Asking him for review is appropriate.
> you can ask glazou, neil, brade, smfr, or myself for reviews
When I started working on editor (I've since moved to <video> about a year ago), I was told that the only person who would respond to review requests for editor was peterv. glazou (the module owner) never responded, peterv did...
If peterv doesn't have the appropriate privileges, he should be given them.
Created attachment 387424 [details] [diff] [review]
The test case for this bug had a bogus </head> tag in place of </title>. (Fix in bug 545405.)
This was checked in July 8, 2009. Should be fixed, right?