Attachment 8498173 Details for Bug 907396

[patch] bug fix for FindPrevious[Next]Sibling

hack_3 (text/plain), 9.47 KB, created by Mats Palmgren (inactive)

(hide)

Description:

Filename:

MIME Type:

Creator: Mats Palmgren (inactive)

Size: 9.47 KB

patch

obsolete

># HG changeset patch
># Parent c6971e448e68c211dd3493fe221030bda22ce788
># User Mats Palmgren <mats@mozilla.com>
>
>diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp
>--- a/layout/base/nsCSSFrameConstructor.cpp
>+++ b/layout/base/nsCSSFrameConstructor.cpp
>@@ -6344,20 +6344,43 @@ nsCSSFrameConstructor::IsValidSibling(ns
> 
>   return true;
> }
> 
> nsIFrame*
> nsCSSFrameConstructor::FindFrameForContentSibling(nsIContent* aContent,
>                                                   nsIContent* aTargetContent,
>                                                   uint8_t& aTargetContentDisplay,
>+                                                  nsContainerFrame* aParentFrame,
>                                                   bool aPrevSibling)
> {
>   nsIFrame* sibling = aContent->GetPrimaryFrame();
>-  if (!sibling || sibling->GetContent() != aContent) {
>+  if (!sibling && GetDisplayContentsStyleFor(aContent)) {
>+    // A display:contents node - check if it has a ::before / ::after frame...
>+    sibling = aPrevSibling ?
>+      nsLayoutUtils::GetAfterFrameForContent(aParentFrame, aContent) :
>+      nsLayoutUtils::GetBeforeFrameForContent(aParentFrame, aContent);
>+    if (!sibling) {
>+      // ... then recurse into children ...
>+      const bool forward = !aPrevSibling;
>+      FlattenedChildIterator iter(aContent, forward);
>+      sibling = aPrevSibling ?
>+        FindPreviousSibling(iter, aTargetContent, aTargetContentDisplay, aParentFrame) :
>+        FindNextSibling(iter, aTargetContent, aTargetContentDisplay, aParentFrame);
>+    }
>+    if (!sibling) {
>+      // ... then ::after / ::before on the opposite end.
>+      sibling = aPrevSibling ?
>+        nsLayoutUtils::GetBeforeFrameForContent(aParentFrame, aContent) :
>+        nsLayoutUtils::GetAfterFrameForContent(aParentFrame, aContent);
>+    }
>+    if (!sibling) {
>+      return nullptr;
>+    }
>+  } else if (!sibling || sibling->GetContent() != aContent) {
>     // XXX the GetContent() != aContent check is needed due to bug 135040.
>     // Remove it once that's fixed.
>     return nullptr;
>   }
> 
>   // If the frame is out-of-flow, GetPrimaryFrame() will have returned the
>   // out-of-flow frame; we want the placeholder.
>   if (sibling->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
>@@ -6386,44 +6409,46 @@ nsCSSFrameConstructor::FindFrameForConte
>     sibling = nullptr;
>   }
> 
>   return sibling;
> }
> 
> nsIFrame*
> nsCSSFrameConstructor::FindPreviousSibling(FlattenedChildIterator aIter,
>-                                           uint8_t& aTargetContentDisplay)
>-{
>-  nsIContent* child = aIter.Get();
>-
>+                                           nsIContent* aChild,
>+                                           uint8_t& aTargetContentDisplay,
>+                                           nsContainerFrame* aParentFrame)
>+{
>   // Note: not all content objects are associated with a frame (e.g., if it's
>-  // `display: none') so keep looking until we find a previous frame
>+  // `display: none') so keep looking until we find a previous frame.
>   while (nsIContent* sibling = aIter.GetPreviousChild()) {
>+    MOZ_ASSERT(sibling != aChild);
>     nsIFrame* prevSibling =
>-      FindFrameForContentSibling(sibling, child, aTargetContentDisplay, true);
>-
>+      FindFrameForContentSibling(sibling, aChild, aTargetContentDisplay,
>+                                 aParentFrame, true);
>     if (prevSibling) {
>       // Found a previous sibling, we're done!
>       return prevSibling;
>     }
>   }
> 
>   return nullptr;
> }
> 
> nsIFrame*
> nsCSSFrameConstructor::FindNextSibling(FlattenedChildIterator aIter,
>-                                       uint8_t& aTargetContentDisplay)
>-{
>-  nsIContent* child = aIter.Get();
>-
>+                                       nsIContent* aChild,
>+                                       uint8_t& aTargetContentDisplay,
>+                                       nsContainerFrame* aParentFrame)
>+{
>   while (nsIContent* sibling = aIter.GetNextChild()) {
>+    MOZ_ASSERT(sibling != aChild);
>     nsIFrame* nextSibling =
>-      FindFrameForContentSibling(sibling, child, aTargetContentDisplay, false);
>+      FindFrameForContentSibling(sibling, aChild, aTargetContentDisplay, aParentFrame, false);
> 
>     if (nextSibling) {
>       // We found a next sibling, we're done!
>       return nextSibling;
>     }
>   }
> 
>   return nullptr;
>@@ -6485,30 +6510,32 @@ nsCSSFrameConstructor::GetInsertionPrevS
>     iter.GetNextChild();
>     NS_WARNING("Someone passed native anonymous content directly into frame "
>                "construction.  Stop doing that!");
>   }
> 
>   // Note that FindPreviousSibling is passed the iterator by value, so that
>   // the later usage of the iterator starts from the same place.
>   uint8_t childDisplay = UNSET_DISPLAY;
>-  nsIFrame* prevSibling = FindPreviousSibling(iter, childDisplay);
>+  nsIFrame* prevSibling =
>+    FindPreviousSibling(iter, iter.Get(), childDisplay, aInsertion->mParentFrame);
> 
>   // Now, find the geometric parent so that we can handle
>   // continuations properly. Use the prev sibling if we have it;
>   // otherwise use the next sibling.
>   if (prevSibling) {
>     aInsertion->mParentFrame = prevSibling->GetParent()->GetContentInsertionFrame();
>   } else {
>     // If there is no previous sibling, then find the frame that follows
>     if (aEndSkipChild) {
>       iter.Seek(aEndSkipChild);
>       iter.GetPreviousChild();
>     }
>-    nsIFrame* nextSibling = FindNextSibling(iter, childDisplay);
>+    nsIFrame* nextSibling =
>+      FindNextSibling(iter, iter.Get(), childDisplay, aInsertion->mParentFrame);
>     if (boxContents) {
>       if (!nextSibling) {
>         // Our siblings (if any) does not have a frame to guide us.
>         // The frame for aChild should be inserted whereever a frame for
>         // the container would be inserted.  This is needed when inserting
>         // into nested display:contents nodes.
>         nsIContent* child = aInsertion->mContainer;
>         InsertionPoint fakeInsertion(aInsertion->mParentFrame, child->GetParent());
>@@ -6516,22 +6543,22 @@ nsCSSFrameConstructor::GetInsertionPrevS
>                                                    aIsRangeInsertSafe, nullptr, nullptr);
>         MOZ_ASSERT(aInsertion->mParentFrame == fakeInsertion.mParentFrame);
>         return result;
>       }
> 
>       prevSibling = nextSibling->GetPrevSibling();
>       if (prevSibling) {
>         // We have a prev-sibling frame that FindPreviousSibling veto'ed or
>-        // it didn't find because it's not for a content sibling.
>-        // Make a new check in case it's the latter case.
>+        // didn't find because it's not for a content sibling.  Make a new
>+        // check in case it's the latter case.
>         prevSibling =
>           FindFrameForContentSibling(prevSibling->GetContent(),
>                                      nextSibling->GetContent(),
>-                                     childDisplay, true);        
>+                                     childDisplay, aInsertion->mParentFrame, true);        
>       }
>     }
> 
>     if (nextSibling) {
>       aInsertion->mParentFrame = nextSibling->GetParent()->GetContentInsertionFrame();
>     } else {
>       // No previous or next sibling, so treat this like an appended frame.
>       *aIsAppend = true;
>diff --git a/layout/base/nsCSSFrameConstructor.h b/layout/base/nsCSSFrameConstructor.h
>--- a/layout/base/nsCSSFrameConstructor.h
>+++ b/layout/base/nsCSSFrameConstructor.h
>@@ -1785,29 +1785,34 @@ private:
>   // will look for last continuations, etc, as necessary.  This calls
>   // IsValidSibling as needed; if that returns false it returns null.
>   //
>   // @param aTargetContentDisplay the CSS display enum for aTargetContent if
>   // already known, UNSET_DISPLAY otherwise. It will be filled in if needed.
>   nsIFrame* FindFrameForContentSibling(nsIContent* aContent,
>                                        nsIContent* aTargetContent,
>                                        uint8_t& aTargetContentDisplay,
>+                                       nsContainerFrame* aParentFrame,
>                                        bool aPrevSibling);
> 
>   // Find the ``rightmost'' frame for the content immediately preceding the one
>   // aIter points to, following continuations if necessary.  aIter is passed by
>   // value on purpose, so as not to modify the caller's iterator.
>   nsIFrame* FindPreviousSibling(mozilla::dom::FlattenedChildIterator aIter,
>-                                uint8_t& aTargetContentDisplay);
>+                                nsIContent* aChild,
>+                                uint8_t& aTargetContentDisplay,
>+                                nsContainerFrame* aParentFrame);
> 
>   // Find the frame for the content node immediately following the one aIter
>   // points to, following continuations if necessary.  aIter is passed by value
>   // on purpose, so as not to modify the caller's iterator.
>   nsIFrame* FindNextSibling(mozilla::dom::FlattenedChildIterator aIter,
>-                            uint8_t& aTargetContentDisplay);
>+                            nsIContent* aChild,
>+                            uint8_t& aTargetContentDisplay,
>+                            nsContainerFrame* aParentFrame);
> 
>   // Find the right previous sibling for an insertion.  This also updates the
>   // parent frame to point to the correct continuation of the parent frame to
>   // use, and returns whether this insertion is to be treated as an append.
>   // aChild is the child being inserted.
>   // aIsRangeInsertSafe returns whether it is safe to do a range insert with
>   // aChild being the first child in the range. It is the callers'
>   // responsibility to check whether a range insert is safe with regards to

Flags:

bzbarsky: review+

Actions: View | Diff | Review

Attachments on bug 907396: 802681 | 802684 | 802685 | 802686 | 802687 | 802688 | 802689 | 802691 | 802692 | 802694 | 802695 | 805961 | 805966 | 805978 | 805980 | 805982 | 806000 | 806002 | 806173 | 806174 | 806177 | 8392917 | 8392920 | 8392929 | 8392932 | 8392934 | 8392953 | 8392954 | 8408496 | 8442107 | 8442117 | 8442176 | 8442192 | 8455264 | 8455266 | 8455274 | 8455281 | 8455290 | 8455386 | 8455502 | 8455503 | 8457452 | 8492588 | 8492589 | 8492590 | 8492591 | 8492593 | 8492594 | 8492595 | 8492596 | 8492597 | 8492598 | 8492600 | 8492601 | 8492602 | 8492603 | 8493349 | 8493408 | 8493412 | 8493953 | 8493956 | 8493957 | 8493977 | 8498158 | 8498173 | 8498181 | 8498189 | 8498191 | 8498197 | 8507588 | 8509088 | 8509092 | 8509095 | 8509098 | 8521591 | 8521869 | 8521871 | 8521873