Bug 1088489 - Meet the specification for pseudo ruby box generation. r=bz

layout/base/nsCSSFrameConstructor.cpp

@@ -9404,7 +9404,9 @@ nsCSSFrameConstructor::CreateNeededPseudos(nsFrameConstructorState& aState,
           int nextDisplay = -1;
           int prevDisplay = -1;
 
-          if (!endIter.AtStart() && IsRubyParentType(ourParentType)) {
+          if (!endIter.AtStart() &&
+              (IsRubyParentType(ourParentType) ||
+               IsRubyParentType(groupingParentType))) {
             FCItemIterator prevItemIter(endIter);
             prevItemIter.Prev();
             prevDisplay =
@@ -9413,11 +9415,24 @@ nsCSSFrameConstructor::CreateNeededPseudos(nsFrameConstructorState& aState,
 
           // We only need to compute nextDisplay for testing for ruby white
           // space.
-          if (!spaceEndIter.IsDone() && IsRubyParentType(ourParentType)) {
+          if (!spaceEndIter.IsDone() &&
+              (IsRubyParentType(ourParentType) ||
+               IsRubyParentType(groupingParentType))) {
             nextDisplay =
               spaceEndIter.item().mStyleContext->StyleDisplay()->mDisplay;
           }
 
+          if (ourParentType == eTypeRubyBaseContainer &&
+              prevDisplay == -1 && nextDisplay == -1) {
+            if (aParentFrame->StyleContext()->GetPseudo()) {
+              // We are in a pseudo ruby base container, which has
+              // whitespaces only. This is a special case to handle
+              // inter-segment spaces.
+              endIter = spaceEndIter;
+              break;
+            }
+          }
+
           // We drop the whitespace in the following cases:
           // 1) If these are not trailing spaces and the next item wants a table
           //    or table-part parent
@@ -9438,8 +9453,9 @@ nsCSSFrameConstructor::CreateNeededPseudos(nsFrameConstructorState& aState,
             prevDisplay == -1 && isRubyLeadingTrailingParentType;
           bool isRubyTrailing =
             nextDisplay == -1 && isRubyLeadingTrailingParentType;
-          // There's an implicit && "IsRubyParentType(ourParentType)" here
-          // because nextDisplay is only set if this is true.
+          // There's an implicit condition that we are in a ruby parent or
+          // we are grouping a ruby parent here, because nextDisplay and
+          // prevDisplay are only set if that is true.
           bool isRubyInterLevel =
             (nextDisplay == NS_STYLE_DISPLAY_RUBY_TEXT_CONTAINER) ||
             (nextDisplay == NS_STYLE_DISPLAY_RUBY_TEXT &&
@@ -9475,10 +9491,25 @@ nsCSSFrameConstructor::CreateNeededPseudos(nsFrameConstructorState& aState,
         // we'll traverse this whitespace again.  But it'll all just be quick
         // DesiredParentType() checks which will match ourParentType (that's
         // what it means that this is the group end), so it's OK.
+        // However, when we are grouping a ruby parent, and endIter points to
+        // a non-droppable whitespace, if the next non-whitespace item also
+        // wants a ruby parent which is not our parent, the whitespace should
+        // also be included into the current ruby parent.
         prevParentType = endIter.item().DesiredParentType();
         if (prevParentType == ourParentType) {
-          // End the group at endIter.
-          break;
+          if (endIter == spaceEndIter ||
+              // not grouping a ruby parent
+              !IsRubyParentType(groupingParentType) ||
+              spaceEndIter.IsDone()) {
+            // End the group at endIter.
+            break;
+          }
+          ParentType nextParentType = spaceEndIter.item().DesiredParentType();
+          if (nextParentType == ourParentType ||
+              !IsRubyParentType(nextParentType)) {
+            // End the group at endIter.
+            break;
+          }
         }
 
         if (ourParentType == eTypeTable &&
@@ -9489,11 +9520,29 @@ nsCSSFrameConstructor::CreateNeededPseudos(nsFrameConstructorState& aState,
           break;
         }
 
-        // Don't group ruby base boxes and ruby annotation boxes together
-        if (ourParentType == eTypeRuby &&
-            (prevParentType == eTypeRubyTextContainer) !=
-            (groupingParentType == eTypeRubyTextContainer)) {
-          break;
+        // Break from the boundary between a ruby base container and a
+        // ruby text container, or the boundary between an inter-segment
+        // whitespace and the next ruby segment.
+        if (ourParentType == eTypeRuby) {
+          if ((prevParentType == eTypeRubyBaseContainer &&
+               groupingParentType == eTypeRubyTextContainer) ||
+              (prevParentType == eTypeRubyTextContainer &&
+               groupingParentType == eTypeRubyBaseContainer)) {
+            // Don't group ruby base boxes and
+            // ruby annotation boxes together.
+            break;
+          } else if (groupingParentType == eTypeBlock &&
+                     endIter != spaceEndIter) {
+            // We are on inter-segment whitespaces, which we want to
+            // create an independent ruby base container for.
+            endIter = spaceEndIter;
+            break;
+          }
+          // The only case where prevParentType is different from
+          // groupingParentType but we still want to continue, is that
+          // we are on an inter-base or inter-annotation whitespace.
+          MOZ_ASSERT(groupingParentType == prevParentType ||
+                     prevParentType == eTypeBlock);
         }
 
         // If we have some whitespace that we were not able to drop and there is
@@ -9543,6 +9592,10 @@ nsCSSFrameConstructor::CreateNeededPseudos(nsFrameConstructorState& aState,
         if (groupingParentType == eTypeRubyTextContainer) {
           wrapperType = eTypeRubyTextContainer;
         } else {
+          NS_ASSERTION(groupingParentType == eTypeRubyBaseContainer ||
+                       groupingParentType == eTypeBlock,
+                       "It should be either a ruby base container, "
+                       "or an inter-segment whitespace");
           wrapperType = eTypeRubyBaseContainer;
         } 
         break;

layout/reftests/css-ruby/reftest.list

@@ -1,6 +1,6 @@
 default-preferences pref(layout.css.ruby.enabled,true)
 
-fails asserts(3-7) == ruby-whitespace-1.html ruby-whitespace-1-ref.html # bug 1052924
+asserts(3-20) == ruby-whitespace-1.html ruby-whitespace-1-ref.html # bug 1052924
 == ruby-whitespace-2.html ruby-whitespace-2-ref.html
 asserts(0-1) != ruby-reflow-1-opaqueruby.html ruby-reflow-1-noruby.html # bug 1052924
 asserts(0-1) == ruby-reflow-1-transparentruby.html ruby-reflow-1-noruby.html # bug 1052924

layout/reftests/css-ruby/ruby-whitespace-1-ref.html

@@ -2,6 +2,7 @@
 <meta charset="UTF-8">
 <head>
 <style>
+body { line-height: 5em; }
 ruby { display: ruby; }
 rb   { display: ruby-base; white-space: nowrap; }
 rp   { display: none; }
@@ -13,23 +14,29 @@ ruby, rb, rt, rbc, rtc { unicode-bidi: isolate; }
 </head>
 <body>
 
-<!-- It's unclear what should happen to the whitespace between the </rtc> and
-     the <rbc>. A literal application of the spec suggests: it gets converted to
-     its own ruby segment, since it is "inter-segment" white space. But then
-     it's a segment containing only trailing whitespace, which gets removed.
-     This results in an empty segment with no whitespace, which doesn't seem
-     correct. //-->
-<ruby><rbc><rb><span>   </span></rb><rb><span>    </span></rb><rb
-  >Base three</rb></rbc><rtc><rt></rt><rt>Text two</rt></rtc><rtc
-  ><rt></rt></rtc><rbc><span>        </span></rbc><rbc><rb
-  >Segment two</rb></rbc><rtc><rt></rt></rtc></ruby>
+<p>
+  <ruby><rbc><rb><span>   </span></rb><rb><span>    </span></rb><rb>Base three</rb></rbc
+    ><rtc><rt><span>   </span></rt><rt><span>   </span></rt><rt>Text three</rt></rtc
+    ><rtc><rt></rt></rtc
+    ><rbc><rb><span>        </span></rb></rbc
+    ><rbc><rb>Segment two</rb></rbc
+    ><rtc><rt></rt></rtc></ruby>
+</p>
 
-<ruby><rbc><rb><span>  </span></rb>Base two<rb><span>    </span></rb><rb></rb
-  ></rbc><rtc><rt><span>  </span></rt>Text two<rt><span>     </span></rt><rt
-  ></rt></rtc></ruby>
+<p>
+  <ruby><rbc><rb>Base one</rb><rb><span>  </span></rb><rb>Base three</rb></rbc
+    ><rtc><rt>Text one</rt><rt><span>  </span></rt><rt>Text three</rt></rtc></ruby>
+</p>
 
-<ruby><rbc><rb><span>   </span></rb><rb>Base two</rb></rbc><rtc><rt> </rt></rtc
-  ></ruby>
+<p>
+  <ruby><rbc><rb>Segment one</rb></rbc
+    ><rbc><rb><span>  </span></rb></rbc
+    ><rbc><rb><span>  </span></rb><rb><span>    </span></rb><rb>Base three</rb></rbc
+    ><rtc><rt><span>  </span></rt><rt><span>     </span></rt><rt>Text three</rt></rtc
+    ><rbc><rb><span>  </span></rb></rbc
+    ><rbc><rb>Base one</rb><rb><span>   </span></rb><rb>Base three</rb></rbc
+    ><rtc><rt>Text one</rt><rt>Text two/three</rt></rtc></ruby>
+</p>
 
 </body>
 </html>

layout/reftests/css-ruby/ruby-whitespace-1.html

@@ -2,6 +2,7 @@
 <meta charset="UTF-8">
 <head>
 <style>
+body { line-height: 5em; }
 ruby { display: ruby; }
 rb   { display: ruby-base; white-space: nowrap; }
 rp   { display: none; }
@@ -13,16 +14,27 @@ ruby, rb, rt, rbc, rtc { unicode-bidi: isolate; }
 </head>
 <body>
 
-<ruby>
-  <rbc>       <rb>   </rb>    <rb>Base three</rb>   </rbc> 
-      <rtc>    <rt>   </rt>   <rt>Text two</rt>   </rtc>    <rtc><rt></rt></rtc>
-        <rbc><rb>Segment two</rb></rbc><rtc><rt></rt></rtc>
-</ruby>
+<p>
+  <ruby>
+    <rbc>       <rb>   </rb>    <rb>Base three</rb>   </rbc> 
+    <rtc>    <rt>   </rt>   <rt>Text three</rt>   </rtc>    <rtc><rt></rt></rtc>
+    <rbc><rb>Segment two</rb></rbc><rtc><rt></rt></rtc>
+  </ruby>
+</p>
 
-<rbc>   <rb>  </rb>    <rb>Base two</rb>  </rbc>   <rtc><rt>  </rt>     <rt
-  >Text two</rt></rtc>
+<p>
+  <ruby>
+    <rb>Base one</rb>  <rb>Base three</rb>
+    <rt>Text one</rt>  <rt>Text three</rt>
+  </ruby>
+</p>
 
-<rb></rb>   <rb>Base two</rb>   <rt> </rt>
+<p>
+  <rb>Segment one</rb>  <rbc>   <rb>  </rb>    <rb>Base three</rb>  </rbc
+    >   <rtc><rt>  </rt>     <rt>Text three</rt></rtc>
+
+  <rb>Base one</rb>   <rb>Base three</rb>   <rt>Text one</rt><rt>Text two/three</rt>
+</p>
 
 </body>
 </html>