Bug 830192 - Integrate GetFixedContainingBlock into GetAbsoluteContainingBlock and ensure only elements which can be abs-pos containing blocks are turned into fixed-pos containing blocks when transformed. r=bz, a=akeybl
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 25 Jan 2013 21:58:33 +1300
changeset 127372 c8641ce92c72edd5a17b74cde60c3f0049daef7e
parent 127371 71e945235fd2c4333ac2b0617bd34c6fac93c0dc
child 127373 1ae5eca14cfed3e97fe70c7358aed65082d41998
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, akeybl
bugs830192
milestone20.0a2
Bug 830192 - Integrate GetFixedContainingBlock into GetAbsoluteContainingBlock and ensure only elements which can be abs-pos containing blocks are turned into fixed-pos containing blocks when transformed. r=bz, a=akeybl
layout/base/crashtests/830138-1.html
layout/base/crashtests/830192-1.html
layout/base/crashtests/830299-1.html
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/base/nsPresShell.cpp
layout/reftests/transform/reftest.list
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/830138-1.html
@@ -0,0 +1,17 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<body>
+<math><menclose id="m" style="transform:translate(10px,0)">
+  <ll style="display:block">
+    <ll id=test2 style="display:none; position: fixed"></ll>
+  </ll>
+</menclose></math>
+<script>
+function doTest() {
+  document.getElementById("test2").setAttribute("style", "position:fixed")
+  document.documentElement.removeAttribute("class");
+}
+window.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/830192-1.html
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<style>
+.test {
+  position:fixed;
+  display:none;
+  width:100px; height:100px;
+  background:yellow;
+}
+.doTest .test {
+  display:block;
+}
+</style>
+</head>
+<body>
+<table>
+<tr style="transform:translate(10px,0)">
+<td>
+  <div class="test"></div>
+</td>
+</tr>
+</table>
+<script>
+function doTest() {
+  document.documentElement.setAttribute("class", "doTest");
+}
+window.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/830299-1.html
@@ -0,0 +1,27 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<style>
+.test {
+  position:fixed;
+  display:none;
+  width:100px; height:100px;
+  background:yellow;
+}
+.doTest .test {
+  display:block;
+}
+</style>
+</head>
+<body>
+<div style="transform:translate(10px,0); overflow:scroll; width:200px; height:200px;">
+  <div class="test"></div>
+</div>
+<script>
+function doTest() {
+  document.documentElement.setAttribute("class", "doTest");
+}
+window.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</body>
+</html>
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -923,19 +923,20 @@ nsFrameConstructorState::nsFrameConstruc
     mPopupItems(nullptr),
 #endif
     mFixedItems(aFixedContainingBlock),
     mAbsoluteItems(aAbsoluteContainingBlock),
     mFloatedItems(aFloatContainingBlock),
     // See PushAbsoluteContaningBlock below
     mFrameState(aHistoryState),
     mAdditionalStateBits(0),
-    mFixedPosIsAbsPos(aAbsoluteContainingBlock &&
-                      aAbsoluteContainingBlock->GetStyleDisplay()->
-                        HasTransform(aAbsoluteContainingBlock)),
+    // If the fixed-pos containing block is equal to the abs-pos containing
+    // block, use the abs-pos containing block's abs-pos list for fixed-pos
+	// frames.
+    mFixedPosIsAbsPos(aFixedContainingBlock == aAbsoluteContainingBlock),
     mHavePendingPopupgroup(false),
     mCreatingExtraFrames(false),
     mTreeMatchContext(true, nsRuleWalker::eRelevantLinkUnvisited,
                       aPresShell->GetDocument()),
     mCurrentPendingBindingInsertionPoint(nullptr)
 {
 #ifdef MOZ_XUL
   nsIRootBox* rootBox = nsIRootBox::GetRootBox(aPresShell);
@@ -956,19 +957,20 @@ nsFrameConstructorState::nsFrameConstruc
 #ifdef MOZ_XUL    
     mPopupItems(nullptr),
 #endif
     mFixedItems(aFixedContainingBlock),
     mAbsoluteItems(aAbsoluteContainingBlock),
     mFloatedItems(aFloatContainingBlock),
     // See PushAbsoluteContaningBlock below
     mAdditionalStateBits(0),
-    mFixedPosIsAbsPos(aAbsoluteContainingBlock &&
-                      aAbsoluteContainingBlock->GetStyleDisplay()->
-                        HasTransform(aAbsoluteContainingBlock)),
+    // If the fixed-pos containing block is equal to the abs-pos containing
+    // block, use the abs-pos containing block's abs-pos list for fixed-pos
+	// frames.
+    mFixedPosIsAbsPos(aFixedContainingBlock == aAbsoluteContainingBlock),
     mHavePendingPopupgroup(false),
     mCreatingExtraFrames(false),
     mTreeMatchContext(true, nsRuleWalker::eRelevantLinkUnvisited,
                       aPresShell->GetDocument()),
     mCurrentPendingBindingInsertionPoint(nullptr)
 {
 #ifdef MOZ_XUL
   nsIRootBox* rootBox = nsIRootBox::GetRootBox(aPresShell);
@@ -5587,82 +5589,74 @@ nsCSSFrameConstructor::GetFrameFor(nsICo
 
   NS_ASSERTION(insertionFrame == frame || !frame->IsLeaf(),
     "The insertion frame is the primary frame or the primary frame isn't a leaf");
 
   return insertionFrame;
 }
 
 nsIFrame*
-nsCSSFrameConstructor::GetAbsoluteContainingBlock(nsIFrame* aFrame)
+nsCSSFrameConstructor::GetAbsoluteContainingBlock(nsIFrame* aFrame,
+                                                  ContainingBlockType aType)
 {
   NS_PRECONDITION(nullptr != mRootElementFrame, "no root element frame");
 
   // Starting with aFrame, look for a frame that is absolutely positioned or
-  // relatively positioned
+  // relatively positioned (and transformed, if aType is FIXED)
   for (nsIFrame* frame = aFrame; frame; frame = frame->GetParent()) {
     if (frame->IsFrameOfType(nsIFrame::eMathML)) {
       // If it's mathml, bail out -- no absolute positioning out from inside
       // mathml frames.  Note that we don't make this part of the loop
       // condition because of the stuff at the end of this method...
       return nullptr;
     }
 
     // If the frame is positioned, we will probably return it as the containing
     // block (see the exceptions below).  Otherwise, we'll start looking at the
     // parent frame, unless we're dealing with a scrollframe.
     // Scrollframes are special since they're not positioned, but their
     // scrolledframe might be.  So, we need to check this special case to return
     // the correct containing block (the scrolledframe) in that case.
-    if (!frame->IsPositioned()) {
+    // If we're looking for a fixed-pos containing block and the frame is
+    // not transformed, skip it.
+    if (!frame->IsPositioned() ||
+        (aType == FIXED_POS && !frame->GetStyleDisplay()->HasTransform(frame))) {
       continue;
     }
     nsIFrame* absPosCBCandidate = nullptr;
     if (frame->GetType() == nsGkAtoms::scrollFrame) {
       nsIScrollableFrame* scrollFrame = do_QueryFrame(frame);
       absPosCBCandidate = scrollFrame->GetScrolledFrame();
     } else {
       // Only first continuations can be containing blocks.
       absPosCBCandidate = frame->GetFirstContinuation();
     }
     // Is the frame really an absolute container?
     if (!absPosCBCandidate || !absPosCBCandidate->IsAbsoluteContainer()) {
       continue;
     }
 
-    // For tables, return the outer table frame.
+    // For tables, skip the inner frame and consider the outer table frame.
     if (absPosCBCandidate->GetType() == nsGkAtoms::tableFrame) {
-      return absPosCBCandidate->GetParent();
+      continue;
     }
     // For outer table frames, we can just return absPosCBCandidate.
     return absPosCBCandidate;
   }
 
   // It is possible for the search for the containing block to fail, because
   // no absolute container can be found in the parent chain.  In those cases,
   // we fall back to the document element's containing block.
+  if (aType == FIXED_POS) {
+    return mFixedContainingBlock;
+  }
   return mHasRootAbsPosContainingBlock ? mDocElementContainingBlock : nullptr;
 }
 
 nsIFrame*
-nsCSSFrameConstructor::GetFixedContainingBlock(nsIFrame* aFrame)
-{
-  NS_PRECONDITION(nullptr != mRootElementFrame, "no root element frame");
-
-  // Starting with aFrame, look for a frame that is CSS-transformed
-  for (nsIFrame* frame = aFrame; frame; frame = frame->GetParent()) {
-    if (frame->GetStyleDisplay()->HasTransform(frame)) {
-      return frame;
-    }
-  }
-
-  return mFixedContainingBlock;
-}
-
-nsIFrame*
 nsCSSFrameConstructor::GetFloatContainingBlock(nsIFrame* aFrame)
 {
   // Starting with aFrame, look for a frame that is a float containing block.
   // IF we hit a mathml frame, bail out; we don't allow floating out of mathml
   // frames, because they don't seem to be able to deal.
   // The logic here needs to match the logic in ProcessChildren()
   for (nsIFrame* containingBlock = aFrame;
        containingBlock &&
@@ -6646,18 +6640,19 @@ nsCSSFrameConstructor::ContentAppended(n
   // Deal with possible :after generated content on the parent
   nsIFrame* parentAfterFrame;
   parentFrame =
     ::AdjustAppendParentForAfterContent(mPresShell->GetPresContext(),
                                         aContainer, parentFrame,
                                         &parentAfterFrame);
   
   // Create some new frames
-  nsFrameConstructorState state(mPresShell, GetFixedContainingBlock(parentFrame),
-                                GetAbsoluteContainingBlock(parentFrame),
+  nsFrameConstructorState state(mPresShell,
+                                GetAbsoluteContainingBlock(parentFrame, FIXED_POS),
+                                GetAbsoluteContainingBlock(parentFrame, ABS_POS),
                                 GetFloatContainingBlock(parentFrame));
   state.mTreeMatchContext.mAncestorFilter.Init(aContainer->AsElement());
 
   // See if the containing block has :first-letter style applied.
   bool haveFirstLetterStyle = false, haveFirstLineStyle = false;
   nsIFrame* containingBlock = state.mFloatedItems.containingBlock;
   if (containingBlock) {
     haveFirstLetterStyle = HasFirstLetterStyle(containingBlock);
@@ -7081,18 +7076,19 @@ nsCSSFrameConstructor::ContentRangeInser
 
   if (parentFrame->IsFrameOfType(nsIFrame::eMathML)) {
     LAYOUT_PHASE_TEMP_EXIT();
     nsresult rv = RecreateFramesForContent(parentFrame->GetContent(), false);
     LAYOUT_PHASE_TEMP_REENTER();
     return rv;
   }
 
-  nsFrameConstructorState state(mPresShell, GetFixedContainingBlock(parentFrame),
-                                GetAbsoluteContainingBlock(parentFrame),
+  nsFrameConstructorState state(mPresShell,
+                                GetAbsoluteContainingBlock(parentFrame, FIXED_POS),
+                                GetAbsoluteContainingBlock(parentFrame, ABS_POS),
                                 GetFloatContainingBlock(parentFrame),
                                 aFrameState);
   state.mTreeMatchContext.mAncestorFilter.Init(aContainer ?
                                                  aContainer->AsElement() :
                                                  nullptr);
 
   // Recover state for the containing block - we need to know if
   // it has :first-letter or :first-line style applied to it. The
@@ -8697,18 +8693,19 @@ nsCSSFrameConstructor::CreateContinuingT
     nsIFrame* rgNextInFlow = rowGroupFrame->GetNextInFlow();
     if (rgNextInFlow) {
       rowGroupFrame->SetRepeatable(false);
     }
     else if (rowGroupFrame->IsRepeatable()) {
       // Replicate the header/footer frame.
       nsTableRowGroupFrame*   headerFooterFrame;
       nsFrameItems            childItems;
-      nsFrameConstructorState state(mPresShell, mFixedContainingBlock,
-                                    GetAbsoluteContainingBlock(newFrame),
+      nsFrameConstructorState state(mPresShell,
+                                    GetAbsoluteContainingBlock(newFrame, FIXED_POS),
+                                    GetAbsoluteContainingBlock(newFrame, ABS_POS),
                                     nullptr);
       state.mCreatingExtraFrames = true;
 
       headerFooterFrame = static_cast<nsTableRowGroupFrame*>
                                      (NS_NewTableRowGroupFrame(aPresShell, rowGroupFrame->GetStyleContext()));
       nsIContent* headerFooter = rowGroupFrame->GetContent();
       headerFooterFrame->Init(headerFooter, newFrame, nullptr);
       ProcessChildren(state, headerFooter, rowGroupFrame->GetStyleContext(),
@@ -10569,18 +10566,19 @@ nsCSSFrameConstructor::CreateLetterFrame
     // frame.
     // XXXbz it would be really nice to destroy the old frame _first_,
     // then create the new one, so we could avoid this hack.
     aTextContent->SetPrimaryFrame(nullptr);
     nsIFrame* textFrame = NS_NewTextFrame(mPresShell, textSC);
 
     NS_ASSERTION(aBlockContinuation == GetFloatContainingBlock(aParentFrame),
                  "Containing block is confused");
-    nsFrameConstructorState state(mPresShell, mFixedContainingBlock,
-                                  GetAbsoluteContainingBlock(aParentFrame),
+    nsFrameConstructorState state(mPresShell,
+                                  GetAbsoluteContainingBlock(aParentFrame, FIXED_POS),
+                                  GetAbsoluteContainingBlock(aParentFrame, ABS_POS),
                                   aBlockContinuation);
 
     // Create the right type of first-letter frame
     const nsStyleDisplay* display = sc->GetStyleDisplay();
     if (display->IsFloating(aParentFrame)) {
       // Make a floating first-letter frame
       CreateFloatingLetterFrame(state, aBlockFrame, aTextContent, textFrame,
                                 blockContent, aParentFrame, sc, aResult);
@@ -10957,18 +10955,18 @@ nsCSSFrameConstructor::CreateListBoxCont
                                             nsILayoutHistoryState* aFrameState)
 {
 #ifdef MOZ_XUL
   nsresult rv = NS_OK;
 
   // Construct a new frame
   if (nullptr != aParentFrame) {
     nsFrameItems            frameItems;
-    nsFrameConstructorState state(mPresShell, mFixedContainingBlock,
-                                  GetAbsoluteContainingBlock(aParentFrame),
+    nsFrameConstructorState state(mPresShell, GetAbsoluteContainingBlock(aParentFrame, FIXED_POS),
+                                  GetAbsoluteContainingBlock(aParentFrame, ABS_POS),
                                   GetFloatContainingBlock(aParentFrame), 
                                   mTempFrameTreeState);
 
     // If we ever initialize the ancestor filter on |state|, make sure
     // to push the right parent!
 
     nsRefPtr<nsStyleContext> styleContext;
     styleContext = ResolveStyleContext(aParentFrame, aChild, &state);
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -1467,18 +1467,21 @@ private:
    * These two functions are used when we start frame creation from a non-root
    * element. They should recreate the same state that we would have
    * arrived at if we had built frames from the root frame to aFrame.
    * Therefore, any calls to PushFloatContainingBlock and
    * PushAbsoluteContainingBlock during frame construction should get
    * corresponding logic in these functions.
    */
 public:
-  nsIFrame* GetAbsoluteContainingBlock(nsIFrame* aFrame);
-  nsIFrame* GetFixedContainingBlock(nsIFrame* aFrame);
+  enum ContainingBlockType {
+    ABS_POS,
+    FIXED_POS
+  };
+  nsIFrame* GetAbsoluteContainingBlock(nsIFrame* aFrame, ContainingBlockType aType);
 private:
   nsIFrame* GetFloatContainingBlock(nsIFrame* aFrame);
 
   nsIContent* PropagateScrollToViewport();
 
   // Build a scroll frame: 
   //  Calls BeginBuildingScrollFrame, InitAndRestoreFrame, and then FinishBuildingScrollFrame.
   // @param aNewFrame the created scrollframe --- output only
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -8958,17 +8958,18 @@ void ColorToString(nscolor aColor, nsAut
 
   PR_snprintf(buf, sizeof(buf), "#%02x%02x%02x",
               NS_GET_R(aColor), NS_GET_G(aColor), NS_GET_B(aColor));
   CopyASCIItoUTF16(buf, aString);
 }
 
 nsIFrame* nsIPresShell::GetAbsoluteContainingBlock(nsIFrame *aFrame)
 {
-  return FrameConstructor()->GetAbsoluteContainingBlock(aFrame);
+  return FrameConstructor()->GetAbsoluteContainingBlock(aFrame,
+      nsCSSFrameConstructor::ABS_POS);
 }
 
 #ifdef ACCESSIBILITY
 bool
 nsIPresShell::IsAccessibilityActive()
 {
   return GetAccService() != nullptr;
 }
--- a/layout/reftests/transform/reftest.list
+++ b/layout/reftests/transform/reftest.list
@@ -110,17 +110,17 @@ skip-if(B2G) fails-if(Android) fuzzy-if(
 # Some simple checks that it obeys selector operations
 == descendant-1.html descendant-1-ref.html
 == propagate-inherit-boolean.html propagate-inherit-boolean-ref.html
 # Ensure you can't move outside an iframe
 == iframe-1.html iframe-1-ref.html
 # Bugs
 == 601894-1.html 601894-ref.html
 == 601894-2.html 601894-ref.html
-fails asserts(1) == 830299-1.html 830299-1-ref.html
+== 830299-1.html 830299-1-ref.html
 # Bug 722777
 == table-1a.html table-1-ref.html
 == table-1b.html table-1-ref.html
 == table-1c.html table-1-ref.html
 == table-2a.html table-2-ref.html
 == table-2b.html table-2-ref.html
 # Bug 722463
 == inline-1a.html inline-1-ref.html