Bug 1064172 - Prevent shaping across element boundaries when margin/border/padding is present, vertical-align is not 'baseline', or there is a bidi isolation boundary. r=jwatt
authorJonathan Kew <jkew@mozilla.com>
Wed, 05 Dec 2018 00:27:47 -0500
changeset 509306 cfc78c02568f395b46976fb724e40eef9299e628
parent 509305 13c1ffa4ed2b21db98e2064d16c71ea5d6c874e9
child 509307 abad2ed6d90b6cae6cf9368fadd0c5de638ace0e
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1064172
milestone66.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 1064172 - Prevent shaping across element boundaries when margin/border/padding is present, vertical-align is not 'baseline', or there is a bidi isolation boundary. r=jwatt
layout/generic/nsTextFrame.cpp
layout/reftests/css-ruby/intrinsic-isize-1-ref.html
layout/reftests/css-ruby/intrinsic-isize-1.html
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -1823,38 +1823,120 @@ bool BuildTextRunsScanner::ContinueTextR
     FrameBidiData data2 = aFrame2->GetBidiData();
     if (data1.embeddingLevel != data2.embeddingLevel ||
         data2.precedingControl != kBidiLevelNone) {
       return false;
     }
   }
 
   ComputedStyle* sc1 = aFrame1->Style();
+  ComputedStyle* sc2 = aFrame2->Style();
+
+  // Any difference in writing-mode/directionality inhibits shaping across
+  // the boundary.
+  WritingMode wm(sc1);
+  if (wm != WritingMode(sc2)) {
+    return false;
+  }
+
   const nsStyleText* textStyle1 = sc1->StyleText();
   // If the first frame ends in a preformatted newline, then we end the textrun
   // here. This avoids creating giant textruns for an entire plain text file.
   // Note that we create a single text frame for a preformatted text node,
   // even if it has newlines in it, so typically we won't see trailing newlines
   // until after reflow has broken up the frame into one (or more) frames per
   // line. That's OK though.
-  if (textStyle1->NewlineIsSignificant(aFrame1) && HasTerminalNewline(aFrame1))
+  if (textStyle1->NewlineIsSignificant(aFrame1) &&
+      HasTerminalNewline(aFrame1)) {
     return false;
+  }
+
+  if (aFrame1->GetParent()->GetContent() !=
+      aFrame2->GetParent()->GetContent()) {
+    // Does aFrame, or any ancestor between it and aAncestor, have a property
+    // that should inhibit cross-element-boundary shaping on aSide?
+    auto PreventCrossBoundaryShaping = [](const nsIFrame* aFrame,
+                                          const nsIFrame* aAncestor,
+                                          Side aSide) {
+      while (aFrame != aAncestor) {
+        ComputedStyle* ctx = aFrame->Style();
+        // According to https://drafts.csswg.org/css-text/#boundary-shaping:
+        //
+        // Text shaping must be broken at inline box boundaries when any of the
+        // following are true for any box whose boundary separates the two
+        // typographic character units:
+        //
+        // 1. Any of margin/border/padding separating the two typographic
+        //    character units in the inline axis is non-zero.
+        const nsStyleCoord& margin = ctx->StyleMargin()->mMargin.Get(aSide);
+        if (!margin.ConvertsToLength() || margin.ToLength() != 0) {
+          return true;
+        }
+        const nsStyleCoord& padding = ctx->StylePadding()->mPadding.Get(aSide);
+        if (!padding.ConvertsToLength() || padding.ToLength() != 0) {
+          return true;
+        }
+        if (ctx->StyleBorder()->GetComputedBorderWidth(aSide) != 0) {
+          return true;
+        }
+
+        // 2. vertical-align is not baseline.
+        const nsStyleCoord& coord = ctx->StyleDisplay()->mVerticalAlign;
+        if (coord.GetUnit() != eStyleUnit_Enumerated ||
+            coord.GetIntValue() != NS_STYLE_VERTICAL_ALIGN_BASELINE) {
+          return true;
+        }
+
+        // 3. The boundary is a bidi isolation boundary.
+        const uint8_t unicodeBidi = ctx->StyleTextReset()->mUnicodeBidi;
+        if (unicodeBidi == NS_STYLE_UNICODE_BIDI_ISOLATE ||
+            unicodeBidi == NS_STYLE_UNICODE_BIDI_ISOLATE_OVERRIDE) {
+          return true;
+        }
+
+        aFrame = aFrame->GetParent();
+      }
+      return false;
+    };
+
+    const nsIFrame* ancestor =
+      nsLayoutUtils::FindNearestCommonAncestorFrame(aFrame1, aFrame2);
+    MOZ_ASSERT(ancestor);
+
+    // Map inline-end and inline-start to physical sides for checking presence
+    // of non-zero margin/border/padding.
+    Side side1 = wm.PhysicalSide(eLogicalSideIEnd);
+    Side side2 = wm.PhysicalSide(eLogicalSideIStart);
+    // If the frames have an embedding level that is opposite to the writing
+    // mode, we need to swap which sides we're checking.
+    if (IS_LEVEL_RTL(aFrame1->GetEmbeddingLevel()) == wm.IsBidiLTR()) {
+      Swap(side1, side2);
+    }
+
+    if (PreventCrossBoundaryShaping(aFrame1, ancestor, side1) ||
+        PreventCrossBoundaryShaping(aFrame2, ancestor, side2)) {
+      return false;
+    }
+  }
 
   if (aFrame1->GetContent() == aFrame2->GetContent() &&
       aFrame1->GetNextInFlow() != aFrame2) {
     // aFrame2 must be a non-fluid continuation of aFrame1. This can happen
     // sometimes when the unicode-bidi property is used; the bidi resolver
     // breaks text into different frames even though the text has the same
     // direction. We can't allow these two frames to share the same textrun
     // because that would violate our invariant that two flows in the same
     // textrun have different content elements.
     return false;
   }
 
-  ComputedStyle* sc2 = aFrame2->Style();
+  if (sc1 == sc2) {
+    return true;
+  }
+
   const nsStyleText* textStyle2 = sc2->StyleText();
   if (sc1 == sc2) return true;
 
   nsPresContext* pc = aFrame1->PresContext();
   MOZ_ASSERT(pc == aFrame2->PresContext());
 
   const nsStyleFont* fontStyle1 = sc1->StyleFont();
   const nsStyleFont* fontStyle2 = sc2->StyleFont();
--- a/layout/reftests/css-ruby/intrinsic-isize-1-ref.html
+++ b/layout/reftests/css-ruby/intrinsic-isize-1-ref.html
@@ -2,16 +2,18 @@
 <html lang="ja">
 <head>
   <meta charset="UTF-8">
   <title>Bug 1134432 - Intrinsic ISize calculation of ruby</title>
   <style>
     div {
       display: inline-block;
       border: 1px solid black;
+      font-kerning: none; /* disable kerning, because in the reference file
+                             it might occur across <span> boundaries */
     }
     span {
       white-space: nowrap;
     }
   </style>
 </head>
 <body>
   <div style="width: min-content">
--- a/layout/reftests/css-ruby/intrinsic-isize-1.html
+++ b/layout/reftests/css-ruby/intrinsic-isize-1.html
@@ -2,16 +2,18 @@
 <html lang="ja">
 <head>
   <meta charset="UTF-8">
   <title>Bug 1134432 - Intrinsic ISize calculation of ruby</title>
   <style>
     div {
       display: inline-block;
       border: 1px solid black;
+      font-kerning: none; /* disable kerning, because in the reference file
+                             it might occur across <span> boundaries */
     }
   </style>
 </head>
 <body>
   <div style="width: min-content">
     <ruby><rb>ABC<rb>DEF</ruby>
   </div>
   <div style="width: max-content">