Bug 1177600 - Properly adjust table row and cell positions when their containing block-size changes in vertical-rl writing mode. r=dholbert
authorJonathan Kew <jkew@mozilla.com>
Fri, 26 Jun 2015 16:50:21 -0700
changeset 250450 e174d857a802d9a00583633ec3c95bdb083666c0
parent 250449 3bec99cdf27ae761a8a2744f198625b51b79610b
child 250451 645991c76862d7bf87ecf584737e20ba77f64eb1
push id28956
push usercbook@mozilla.com
push dateMon, 29 Jun 2015 12:17:35 +0000
treeherdermozilla-central@e137fc38c431 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1177600
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1177600 - Properly adjust table row and cell positions when their containing block-size changes in vertical-rl writing mode. r=dholbert
layout/tables/nsTableFrame.cpp
layout/tables/nsTableRowFrame.cpp
layout/tables/nsTableRowGroupFrame.cpp
layout/tables/nsTableRowGroupFrame.h
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1823,16 +1823,17 @@ nsTableFrame::Reflow(nsPresContext*     
 
   bool haveDesiredBSize = false;
   SetHaveReflowedColGroups(false);
 
   // Reflow the entire table (pass 2 and possibly pass 3). This phase is necessary during a
   // constrained initial reflow and other reflows which require either a strategy init or balance.
   // This isn't done during an unconstrained reflow, because it will occur later when the parent
   // reflows with a constrained isize.
+  bool fixupKidPositions = false;
   if (NS_SUBTREE_DIRTY(this) ||
       aReflowState.ShouldReflowAllKids() ||
       IsGeometryDirty() ||
       aReflowState.IsBResize()) {
 
     if (aReflowState.ComputedBSize() != NS_UNCONSTRAINEDSIZE ||
         // Also check IsBResize(), to handle the first Reflow preceding a
         // special bsize Reflow, when we've already had a special bsize
@@ -1872,16 +1873,21 @@ nsTableFrame::Reflow(nsPresContext*     
     // if we need to initiate a special bsize reflow, then don't constrain the
     // bsize of the reflow before that
     nscoord availBSize = needToInitiateSpecialReflow
                          ? NS_UNCONSTRAINEDSIZE
                          : aReflowState.AvailableBSize();
 
     ReflowTable(aDesiredSize, aReflowState, availBSize,
                 lastChildReflowed, aStatus);
+    // If ComputedWidth is unconstrained, we may need to fix child positions
+    // later (in vertical-rl mode) due to use of 0 as a fake containerWidth
+    // during ReflowChildren.
+    fixupKidPositions = wm.IsVerticalRL() &&
+      aReflowState.ComputedWidth() == NS_UNCONSTRAINEDSIZE;
 
     // reevaluate special bsize reflow conditions
     if (HasAnyStateBits(NS_FRAME_CONTAINS_RELATIVE_BSIZE)) {
       needToInitiateSpecialReflow = true;
     }
 
     // XXXldb Are all these conditions correct?
     if (needToInitiateSpecialReflow && NS_FRAME_IS_COMPLETE(aStatus)) {
@@ -1921,16 +1927,25 @@ nsTableFrame::Reflow(nsPresContext*     
     aReflowState.ComputedLogicalBorderPadding().IStartEnd(wm);
   if (!haveDesiredBSize) {
     CalcDesiredBSize(aReflowState, aDesiredSize);
   }
   if (IsRowInserted()) {
     ProcessRowInserted(aDesiredSize.BSize(wm));
   }
 
+  if (fixupKidPositions) {
+    // If we didn't already know the containerWidth (and so used zero during
+    // ReflowChildren), then we need to update the block-position of our kids.
+    for (nsIFrame* kid : mFrames) {
+      kid->MovePositionBy(nsPoint(aDesiredSize.Width(), 0));
+      RePositionViews(kid);
+    }
+  }
+
   LogicalMargin borderPadding = GetChildAreaOffset(wm, &aReflowState);
   SetColumnDimensions(aDesiredSize.BSize(wm), wm, borderPadding,
                       aDesiredSize.Width());
   if (NeedToCollapse() &&
       (NS_UNCONSTRAINEDSIZE != aReflowState.AvailableISize())) {
     AdjustForCollapsingRowsCols(aDesiredSize, wm, borderPadding);
   }
 
@@ -2955,24 +2970,21 @@ nsTableFrame::ReflowChildren(nsTableRefl
   aLastChildReflowed = nullptr;
 
   nsIFrame* prevKidFrame = nullptr;
   WritingMode wm = aReflowState.reflowState.GetWritingMode();
   nscoord containerWidth = aReflowState.reflowState.ComputedWidth();
   if (containerWidth == NS_UNCONSTRAINEDSIZE) {
     NS_WARN_IF_FALSE(wm.IsVertical(),
                      "shouldn't have unconstrained width in horizontal mode");
-    if (wm.IsVerticalRL()) {
-      nsHTMLReflowMetrics desiredSize(wm);
-      CalcDesiredBSize(aReflowState.reflowState, desiredSize);
-      containerWidth = desiredSize.Width();
-    } else {
-      // in vertical-lr mode, containerWidth won't actually be used
-      containerWidth = 0;
-    }
+    // We won't know the containerWidth until we've reflowed our contents,
+    // so use zero for now; in vertical-rl mode, this will mean the children
+    // are misplaced in the block-direction, and will need to be moved
+    // rightwards by the true containerWidth once we know it.
+    containerWidth = 0;
   } else {
     containerWidth +=
       aReflowState.reflowState.ComputedPhysicalBorderPadding().LeftRight();
   }
 
   nsPresContext* presContext = PresContext();
   // XXXldb Should we be checking constrained height instead?
   // tables are not able to pull back children from its next inflow, so even
--- a/layout/tables/nsTableRowFrame.cpp
+++ b/layout/tables/nsTableRowFrame.cpp
@@ -323,28 +323,63 @@ nsTableRowFrame::DidResize()
   // Resize and re-align the cell frames based on our row bsize
   nsTableFrame* tableFrame = GetTableFrame();
 
   WritingMode wm = GetWritingMode();
   nsHTMLReflowMetrics desiredSize(wm);
   desiredSize.SetSize(wm, GetLogicalSize(wm));
   desiredSize.SetOverflowAreasToDesiredBounds();
 
+  nscoord containerWidth = mRect.width;
+
   for (nsIFrame* childFrame : mFrames) {
     nsTableCellFrame *cellFrame = do_QueryFrame(childFrame);
     if (cellFrame) {
       nscoord cellBSize = BSize(wm) +
         GetBSizeOfRowsSpannedBelowFirst(*cellFrame, *tableFrame, wm);
 
-      // resize the cell's bsize
+      // If the bsize for the cell has changed, we need to reset it;
+      // and in vertical-rl mode, we need to update the cell's block position
+      // to account for the containerWidth, which may not have been known
+      // earlier, so we always apply it here.
       LogicalSize cellSize = cellFrame->GetLogicalSize(wm);
-      nsRect cellVisualOverflow = cellFrame->GetVisualOverflowRect();
-      if (cellSize.BSize(wm) != cellBSize) {
+      if (cellSize.BSize(wm) != cellBSize || wm.IsVerticalRL()) {
+        nsRect cellOldRect = cellFrame->GetRect();
+        nsRect cellVisualOverflow = cellFrame->GetVisualOverflowRect();
+
+        if (wm.IsVerticalRL()) {
+          // Get the old position of the cell, as we want to preserve its
+          // inline coordinate.
+          LogicalPoint oldPos =
+            cellFrame->GetLogicalPosition(wm, containerWidth);
+
+          // The cell should normally be aligned with the row's block-start,
+          // so set the B component of the position to zero:
+          LogicalPoint newPos(wm, oldPos.I(wm), 0);
+
+          // ...unless relative positioning is in effect, in which case the
+          // cell may have been moved away from the row's block-start
+          if (cellFrame->IsRelativelyPositioned()) {
+            // Find out where the cell would have been without relative
+            // positioning.
+            LogicalPoint oldNormalPos =
+              cellFrame->GetLogicalNormalPosition(wm, containerWidth);
+            // The difference (if any) between oldPos and oldNormalPos reflects
+            // relative positioning that was applied to the cell, and which we
+            // need to incorporate when resetting the position.
+            newPos.B(wm) = oldPos.B(wm) - oldNormalPos.B(wm);
+          }
+
+          if (oldPos != newPos) {
+            cellFrame->SetPosition(wm, newPos, containerWidth);
+            nsTableFrame::RePositionViews(cellFrame);
+          }
+        }
+
         cellSize.BSize(wm) = cellBSize;
-        nsRect cellOldRect = cellFrame->GetRect();
         cellFrame->SetSize(wm, cellSize);
         nsTableFrame::InvalidateTableFrame(cellFrame, cellOldRect,
                                            cellVisualOverflow,
                                            false);
       }
 
       // realign cell content based on the new bsize.  We might be able to
       // skip this if the bsize didn't change... maybe.  Hard to tell.
@@ -848,20 +883,22 @@ nsTableRowFrame::ReflowChildren(nsPresCo
     prevColIndex = cellColIndex + (cellColSpan - 1);
 
     // Reflow the child frame
     nsRect kidRect = kidFrame->GetRect();
     LogicalPoint origKidNormalPosition =
       kidFrame->GetLogicalNormalPosition(wm, containerWidth);
     // All cells' no-relative-positioning position should be snapped to the
     // row's bstart edge.
-    // XXX This currently doesn't hold in vertical-rl mode, which is probably
-    // a bug, but to enable progress with testing I'm temporarily neutering
-    // the assertion in that case (bug 1174711).
-    MOZ_ASSERT(origKidNormalPosition.B(wm) == 0 || wm.IsVerticalRL());
+    // This doesn't hold in vertical-rl mode, where we don't yet know the
+    // correct containerWidth for the row frame. In that case, we'll have to
+    // fix up child positions later, after determining our desiredSize.
+    NS_ASSERTION(origKidNormalPosition.B(wm) == 0 || wm.IsVerticalRL(),
+                 "unexpected kid position");
+
     nsRect kidVisualOverflow = kidFrame->GetVisualOverflowRect();
     LogicalPoint kidPosition(wm, iCoord, 0);
     bool firstReflow = kidFrame->HasAnyStateBits(NS_FRAME_FIRST_REFLOW);
 
     if (doReflowChild) {
       // Calculate the available isize for the table cell using the known
       // column isizes
       nscoord availCellISize = CalcAvailISize(aTableFrame, *cellFrame);
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -337,17 +337,20 @@ nsTableRowGroupFrame::ReflowChildren(nsP
   // or should we *only* check available block-size?
   // (Think about multi-column layout!)
   bool isPaginated = aPresContext->IsPaginated() &&
                      NS_UNCONSTRAINEDSIZE != aReflowState.availSize.BSize(wm);
 
   bool haveRow = false;
   bool reflowAllKids = aReflowState.reflowState.ShouldReflowAllKids() ||
                          tableFrame->IsGeometryDirty();
-  bool needToCalcRowBSizes = reflowAllKids;
+
+  // in vertical-rl mode, we always need the row bsizes in order to
+  // get the necessary containerWidth for placing our kids
+  bool needToCalcRowBSizes = reflowAllKids || wm.IsVerticalRL();
 
   nscoord containerWidth = aReflowState.reflowState.ComputedWidth();
   if (containerWidth == NS_UNCONSTRAINEDSIZE) {
     containerWidth = 0; // we can't position frames correctly in RTL yet,
                         // so they will need to be adjusted later
   } else {
     containerWidth +=
       aReflowState.reflowState.ComputedPhysicalBorderPadding().LeftRight();
@@ -412,17 +415,17 @@ nsTableRowGroupFrame::ReflowChildren(nsP
       // Place the child
       PlaceChild(aPresContext, aReflowState, kidFrame,
                  wm, kidPosition, containerWidth,
                  desiredSize, oldKidRect.GetPhysicalRect(wm, containerWidth),
                  oldKidVisualOverflow);
       aReflowState.bCoord += cellSpacingB;
 
       if (!reflowAllKids) {
-        if (IsSimpleRowFrame(aReflowState.tableFrame, kidFrame)) {
+        if (IsSimpleRowFrame(aReflowState.tableFrame, rowFrame)) {
           // Inform the row of its new bsize.
           rowFrame->DidResize();
           // the overflow area may have changed inflate the overflow area
           const nsStylePosition *stylePos = StylePosition();
           nsStyleUnit unit = stylePos->BSize(wm).GetUnit();
           if (aReflowState.tableFrame->IsAutoBSize(wm) &&
               unit != eStyleUnit_Coord) {
             // Because other cells in the row may need to be aligned
@@ -1576,30 +1579,26 @@ nsTableRowGroupFrame::GetBSizeBasis(cons
     }
   }
 
   return result;
 }
 
 bool
 nsTableRowGroupFrame::IsSimpleRowFrame(nsTableFrame* aTableFrame,
-                                       nsIFrame*     aFrame)
+                                       nsTableRowFrame* aRowFrame)
 {
-  // Make sure it's a row frame and not a row group frame
-  nsTableRowFrame *rowFrame = do_QueryFrame(aFrame);
-  if (rowFrame) {
-    int32_t rowIndex = rowFrame->GetRowIndex();
+  int32_t rowIndex = aRowFrame->GetRowIndex();
 
-    // It's a simple row frame if there are no cells that span into or
-    // across the row
-    int32_t numEffCols = aTableFrame->GetEffectiveColCount();
-    if (!aTableFrame->RowIsSpannedInto(rowIndex, numEffCols) &&
-        !aTableFrame->RowHasSpanningCells(rowIndex, numEffCols)) {
-      return true;
-    }
+  // It's a simple row frame if there are no cells that span into or
+  // across the row
+  int32_t numEffCols = aTableFrame->GetEffectiveColCount();
+  if (!aTableFrame->RowIsSpannedInto(rowIndex, numEffCols) &&
+      !aTableFrame->RowHasSpanningCells(rowIndex, numEffCols)) {
+    return true;
   }
 
   return false;
 }
 
 nsIAtom*
 nsTableRowGroupFrame::GetType() const
 {
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@@ -399,17 +399,17 @@ protected:
                           nsTableRowFrame*&        aFirstTruncatedRow,
                           nscoord&                 aDesiredHeight);
 
   void CreateContinuingRowFrame(nsPresContext& aPresContext,
                                 nsIFrame&      aRowFrame,
                                 nsIFrame**     aContRowFrame);
 
   bool IsSimpleRowFrame(nsTableFrame* aTableFrame,
-                          nsIFrame*     aFrame);
+                        nsTableRowFrame* aRowFrame);
 
   void GetNextRowSibling(nsIFrame** aRowFrame);
 
   void UndoContinuedRow(nsPresContext*   aPresContext,
                         nsTableRowFrame* aRow);
 
 private:
   // border widths in pixels in the collapsing border model