106212 - Make ContentAppended really work with XBL insertion points

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[David Hyatt]

The following patch makes ContentAppended kick ass with XBL insertion points.  
I finally ran into a XUL use case where I needed this fixed (go tab browser!).  
Right now ContentAppended completely screws up if the insertion point contains 
a mixture of content.  With this patch, it works nearly all of the time.

Comment 1

[David Hyatt]

Attachment: Fix up most of the problems.

Comment 2

[David Hyatt]

Attachment: Ye Olde Patch Demonstrating the Mightiness of My Insertion Point Fu

Comment 3

[Chris Waterson]

Comment on attachment 54654 [details] [diff] [review]
Ye Olde Patch Demonstrating the Mightiness of My Insertion Point Fu

Looks good; sr=waterson

Comment 4

[Chris Waterson]

*** Bug 99543 has been marked as a duplicate of this bug. ***

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 54654 [details] [diff] [review]
Ye Olde Patch Demonstrating the Mightiness of My Insertion Point Fu

-    for (PRInt32 i = mObservers.Count() - 1; i >= 0; --i) {
+    for (PRInt32 i = 0; i < mObservers.Count(); i++) {

I bet the compiler has to reload mImpl, test non-null, if so reload mImpl->mCount
every time through the loop.  Better to store mObservers.Count() in a local PRInt32 n
and loop while i < n.

Also, declare i outside of the for loop control (same goes for the later, inner-block
for loop).

Style nit: isn't it better to indent the condition and update to line up with the
for loop's init part (so as to underhang in the column after the opening paren)?

+        for (ChildIterator::Init(insertionContent, &iter, &last);
+         iter != last;
+         ++iter) {

Otherwise looks good -- do something about those for loops and r=[e-mail]

/be

Comment 6

[David Hyatt]

Fixed.

Comment 7

[R.K.Aa.]

Built on linux from cvs shortly before this checkin: all was well.
But right now it crashes. Mailed Hyatt backtrace from a non-debug.

Comment 8

[Markus Gerstel]

Connection to Bug 106646 ?