Bug 1116631 - Fix isize reservation of ruby base container. r=roc
authorXidorn Quan <quanxunzhen@gmail.com>
Wed, 07 Jan 2015 12:47:09 +1100
changeset 248157 80f15029ae68eef0d3b299097284780c1bab1a91
parent 248156 36ff8406cca7ceb5cd939a003e7cb56a7a281bd0
child 248158 6af6f0cea6fb8adc2534b6b0ff681fc790adf574
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1116631
milestone37.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 1116631 - Fix isize reservation of ruby base container. r=roc
layout/generic/nsRubyBaseContainerFrame.cpp
layout/generic/nsRubyFrame.cpp
layout/reftests/css-ruby/justification-1-ref.html
layout/reftests/css-ruby/justification-1.html
layout/reftests/css-ruby/reftest.list
--- a/layout/generic/nsRubyBaseContainerFrame.cpp
+++ b/layout/generic/nsRubyBaseContainerFrame.cpp
@@ -366,55 +366,54 @@ nsRubyBaseContainerFrame::Reflow(nsPresC
   nscoord isize = 0;
   if (aStatus == NS_FRAME_COMPLETE) {
     // Reflow columns excluding any span
     ReflowState reflowState = {
       allowLineBreak && !hasSpan, textContainers, aReflowState, reflowStates
     };
     isize = ReflowColumns(reflowState, aStatus);
   }
+  DebugOnly<nscoord> lineSpanSize = aReflowState.mLineLayout->EndSpan(this);
+  aDesiredSize.ISize(lineWM) = isize;
+  // When there are no frames inside the ruby base container, EndSpan
+  // will return 0. However, in this case, the actual width of the
+  // container could be non-zero because of non-empty ruby annotations.
+  MOZ_ASSERT(NS_INLINE_IS_BREAK_BEFORE(aStatus) ||
+             isize == lineSpanSize || mFrames.IsEmpty());
 
   // If there exists any span, the columns must either be completely
   // reflowed, or be not reflowed at all.
   MOZ_ASSERT(NS_INLINE_IS_BREAK_BEFORE(aStatus) ||
              NS_FRAME_IS_COMPLETE(aStatus) || !hasSpan);
   if (!NS_INLINE_IS_BREAK_BEFORE(aStatus) &&
       NS_FRAME_IS_COMPLETE(aStatus) && hasSpan) {
     // Reflow spans
     ReflowState reflowState = {
       false, textContainers, aReflowState, reflowStates
     };
     nscoord spanISize = ReflowSpans(reflowState);
     nscoord deltaISize = spanISize - isize;
-    if (deltaISize <= 0) {
-      RubyUtils::ClearReservedISize(this);
-    } else if (allowLineBreak && ShouldBreakBefore(aReflowState, deltaISize)) {
-      aStatus = NS_INLINE_LINE_BREAK_BEFORE();
-    } else {
-      RubyUtils::SetReservedISize(this, deltaISize);
-      aReflowState.mLineLayout->AdvanceICoord(deltaISize);
-      isize = spanISize;
+    if (deltaISize > 0) {
+      if (allowLineBreak && ShouldBreakBefore(aReflowState, deltaISize)) {
+        aStatus = NS_INLINE_LINE_BREAK_BEFORE();
+      } else {
+        isize = spanISize;
+      }
     }
     // When there are spans, ReflowColumns and ReflowOneColumn won't
     // record any optional break position. We have to record one
     // at the end of this segment.
     if (!NS_INLINE_IS_BREAK(aStatus) && allowLineBreak &&
         aReflowState.mLineLayout->NotifyOptionalBreakPosition(
           this, INT32_MAX, startEdge + isize <= aReflowState.AvailableISize(),
           gfxBreakPriority::eNormalBreak)) {
       aStatus = NS_INLINE_LINE_BREAK_AFTER(aStatus);
     }
   }
 
-  DebugOnly<nscoord> lineSpanSize = aReflowState.mLineLayout->EndSpan(this);
-  // When there are no frames inside the ruby base container, EndSpan
-  // will return 0. However, in this case, the actual width of the
-  // container could be non-zero because of non-empty ruby annotations.
-  MOZ_ASSERT(NS_INLINE_IS_BREAK_BEFORE(aStatus) ||
-             isize == lineSpanSize || mFrames.IsEmpty());
   for (uint32_t i = 0; i < rtcCount; i++) {
     // It happens before the ruby text container is reflowed, and that
     // when it is reflowed, it will just use this size.
     nsRubyTextContainerFrame* textContainer = textContainers[i];
     nsLineLayout* lineLayout = lineLayouts[i].get();
 
     RubyUtils::ClearReservedISize(textContainer);
     nscoord rtcISize = lineLayout->GetCurrentICoord();
@@ -424,22 +423,21 @@ nsRubyBaseContainerFrame::Reflow(nsPresC
     // own size.
     if (!textContainer->IsSpanContainer()) {
       rtcISize = isize;
     } else if (isize > rtcISize) {
       RubyUtils::SetReservedISize(textContainer, isize - rtcISize);
     }
 
     lineLayout->VerticalAlignLine();
-    LogicalSize lineSize(lineWM, isize, lineLayout->GetFinalLineBSize());
+    LogicalSize lineSize(lineWM, rtcISize, lineLayout->GetFinalLineBSize());
     aReflowState.mRubyReflowState->SetTextContainerInfo(i, textContainer, lineSize);
     lineLayout->EndLineReflow();
   }
 
-  aDesiredSize.ISize(lineWM) = isize;
   nsLayoutUtils::SetBSizeFromFontMetrics(this, aDesiredSize, aReflowState,
                                          borderPadding, lineWM, frameWM);
 }
 
 /**
  * This struct stores the continuations after this frame and
  * corresponding text containers. It is used to speed up looking
  * ahead for nonempty continuations.
--- a/layout/generic/nsRubyFrame.cpp
+++ b/layout/generic/nsRubyFrame.cpp
@@ -303,16 +303,17 @@ nsRubyFrame::ReflowSegment(nsPresContext
     for (uint32_t i = 0; i < rtcCount; i++) {
       nsIFrame* nextRTC = textContainers[i]->GetNextInFlow();
       if (nextRTC) {
         nextRTC->GetParent()->DeleteNextInFlowChild(nextRTC, true);
       }
     }
   }
 
+  nscoord segmentISize = baseMetrics.ISize(lineWM);
   nsRect baseRect = aBaseContainer->GetRect();
   // We need to position our rtc frames on one side or the other of the
   // base container's rect, using a coordinate space that's relative to
   // the ruby frame. Right now, the base container's rect's block-axis
   // position is relative to the block container frame containing the
   // lines, so we use 0 instead. (i.e. we assume that the base container
   // is adjacent to the ruby frame's block-start edge.)
   // XXX We may need to add border/padding here. See bug 1055667.
@@ -335,21 +336,24 @@ nsRubyFrame::ReflowSegment(nsPresContext
     textReflowState.mLineLayout = aReflowState.mLineLayout;
     textContainer->Reflow(aPresContext, textMetrics,
                           textReflowState, textReflowStatus);
     // Ruby text containers always return NS_FRAME_COMPLETE even when
     // they have continuations, because the breaking has already been
     // handled when reflowing the base containers.
     NS_ASSERTION(textReflowStatus == NS_FRAME_COMPLETE,
                  "Ruby text container must not break itself inside");
-    textContainer->SetSize(LogicalSize(lineWM, textMetrics.ISize(lineWM),
-                                       textMetrics.BSize(lineWM)));
+    nscoord isize = textMetrics.ISize(lineWM);
+    nscoord bsize = textMetrics.BSize(lineWM);
+    textContainer->SetSize(LogicalSize(lineWM, isize, bsize));
+
+    nscoord reservedISize = RubyUtils::GetReservedISize(textContainer);
+    segmentISize = std::max(segmentISize, isize + reservedISize);
 
     nscoord x, y;
-    nscoord bsize = textMetrics.BSize(lineWM);
     uint8_t rubyPosition = textContainer->StyleText()->mRubyPosition;
 #ifdef DEBUG
     SanityCheckRubyPosition(rubyPosition);
 #endif
     if (lineWM.IsVertical()) {
       // writing-mode is vertical, so bsize is the annotation's *width*
       if (rubyPosition & NS_STYLE_RUBY_POSITION_LEFT) {
         x = offsetRect.X() - bsize;
@@ -372,16 +376,24 @@ nsRubyFrame::ReflowSegment(nsPresContext
         // XXX inter-character support in bug 1055672
         MOZ_ASSERT_UNREACHABLE("Unsupported ruby-position");
         y = offsetRect.Y();
       }
     }
     FinishReflowChild(textContainer, aPresContext, textMetrics,
                       &textReflowState, x, y, 0);
   }
+
+  nscoord deltaISize = segmentISize - baseMetrics.ISize(lineWM);
+  if (deltaISize <= 0) {
+    RubyUtils::ClearReservedISize(aBaseContainer);
+  } else {
+    RubyUtils::SetReservedISize(aBaseContainer, deltaISize);
+    aReflowState.mLineLayout->AdvanceICoord(deltaISize);
+  }
 }
 
 nsRubyBaseContainerFrame*
 nsRubyFrame::PullOneSegment(ContinuationTraversingState& aState)
 {
   // Pull a ruby base container
   nsIFrame* baseFrame = PullNextInFlowChild(aState);
   if (!baseFrame) {
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-ruby/justification-1-ref.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>Bug 1116631 - Justification with ruby span</title>
+  <link rel="stylesheet" href="common.css">
+  <style>
+    rtc { line-height: 0; }
+  </style>
+</head>
+<body>
+  <div style="width: 200px; height: 100px; -moz-text-align-last: justify; overflow: auto;"><ruby>
+    <rb>a</rb><rb>b</rb>
+  </ruby></div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-ruby/justification-1.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>Bug 1116631 - Justification with ruby span</title>
+  <link rel="stylesheet" href="common.css">
+  <style>
+    rtc { line-height: 0; }
+  </style>
+</head>
+<body>
+  <div style="width: 200px; height: 100px; -moz-text-align-last: justify; overflow: auto;"><ruby>
+    <rb>a</rb><rb>b</rb>
+    <rtc><div style="width: 100px; height: 0;"></div></rtc>
+  </ruby></div>
+</body>
+</html>
--- a/layout/reftests/css-ruby/reftest.list
+++ b/layout/reftests/css-ruby/reftest.list
@@ -15,15 +15,16 @@ fails == autohiding-3.html autohiding-3-
 == dynamic-removal-2.html dynamic-removal-2-ref.html
 fuzzy-if(winWidget,28,1) == dynamic-removal-3.html dynamic-removal-3-ref.html # bug 1111891
 == float-handling.html float-handling-ref.html
 == inlinize-blocks-1.html inlinize-blocks-1-ref.html
 == inlinize-blocks-2.html inlinize-blocks-2-ref.html
 == inlinize-blocks-3.html inlinize-blocks-3-ref.html
 == inlinize-blocks-4.html inlinize-blocks-4-ref.html
 == inlinize-blocks-5.html inlinize-blocks-5-ref.html
+== justification-1.html justification-1-ref.html
 == ruby-whitespace-1.html ruby-whitespace-1-ref.html
 == ruby-whitespace-2.html ruby-whitespace-2-ref.html
 == ruby-position-horizontal.html ruby-position-horizontal-ref.html
 pref(layout.css.vertical-text.enabled,true) fails == ruby-position-vertical-lr.html ruby-position-vertical-lr-ref.html # bug 1112474
 pref(layout.css.vertical-text.enabled,true) fails == ruby-position-vertical-rl.html ruby-position-vertical-rl-ref.html # bug 1112474
 != ruby-reflow-1-opaqueruby.html ruby-reflow-1-noruby.html
 == ruby-reflow-1-transparentruby.html ruby-reflow-1-noruby.html