565819 - [FIX]Change RestyleForInsert/Append/Remove to use Element and the new sibling API

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Prereq for bug 562700, and slightly cleaner/faster code.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 445268 [details] [diff] [review]
Like so

nsCSSFrameConstructor.cpp:

>-    for (PRInt32 index = 0; index < aNewIndexInContainer; ++index) {
>+    for (nsIContent* cur = aContainer->GetFirstChild();
>+         cur;
>+         cur = cur->GetNextSibling()) {

Shouldn't this be |cur != aFirstNewContent| instead of |cur| as the condition?

>@@ -11493,38 +11489,47 @@ nsCSSFrameConstructor::RestyleForRemove(

>-    // restyle the previously-last element child if it is before aOldChild
>-    for (PRInt32 index = aContainer->GetChildCount() - 1;
>-         index >= 0; --index) {
>-      nsIContent *content = aContainer->GetChildAt(index);
>+    // restyle the now-last element child if it was before aOldChild
>+    reachedFollowingSibling = PR_FALSE;

Shouldn't reachedFollowingSibling be initialized to aFollowingSibling != nsnull ?

r=dbaron with that

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Shouldn't this be |cur != aFirstNewContent| instead of |cur| as the condition?

Yes.  Good catch.

> Shouldn't reachedFollowingSibling be initialized to aFollowingSibling != nsnull

You mean == nsnull.  Yes.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: With those fixed, and tests that would have caught them

Comment 5

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/93be15b7e54e