Bug 727125 - Lazily compute LineBaselineOffset when needed so it is present after a dynamic change of 'text-decoration'. r=dbaron
authorSusanna Bowen <sgbowen8@gmail.com>
Wed, 18 Jun 2014 12:55:00 -0400
changeset 189468 d00494b1dd572f49a9b9636d44ed0ac5c56c9d83
parent 189467 f6b74b7b4995a64102f8fb6b4c9f51f9384a986b
child 189469 75618ce20f689d6402b59d771e621e789f46e97c
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersdbaron
bugs727125
milestone33.0a1
Bug 727125 - Lazily compute LineBaselineOffset when needed so it is present after a dynamic change of 'text-decoration'. r=dbaron This fixes the positioning of underlines set on a block or its ancestor when drawn on children of a block that have a vertical-align != baseline. The lazy computation is done all at once for all children of a block to avoid O(N^2) searches for the line containing a frame.
layout/generic/nsLineLayout.cpp
layout/generic/nsTextFrame.cpp
layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-1-ref.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-1.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-2-ref.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-2.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-1-ref.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-1.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-2-ref.html
layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-2.html
layout/reftests/text-decoration/reftest.list
--- a/layout/generic/nsLineLayout.cpp
+++ b/layout/generic/nsLineLayout.cpp
@@ -726,16 +726,26 @@ nsLineLayout::ReflowFrame(nsIFrame* aFra
 
 #ifdef REALLY_NOISY_REFLOW
   nsFrame::IndentBy(stdout, mSpanDepth);
   printf("%p: Begin ReflowFrame pfd=%p ", psd, pfd);
   nsFrame::ListTag(stdout, aFrame);
   printf("\n");
 #endif
 
+  if (mCurrentSpan == mRootSpan) {
+    pfd->mFrame->Properties().Remove(nsIFrame::LineBaselineOffset());
+  } else {
+#ifdef DEBUG
+    bool hasLineOffset;
+    pfd->mFrame->Properties().Get(nsIFrame::LineBaselineOffset(), &hasLineOffset);
+    NS_ASSERTION(!hasLineOffset, "LineBaselineOffset was set but was not expected");
+#endif
+  }
+
   mTextJustificationNumSpaces = 0;
   mTextJustificationNumLetters = 0;
 
   // Stash copies of some of the computed state away for later
   // (block-direction alignment, for example)
   WritingMode frameWM = aFrame->GetWritingMode();
   WritingMode lineWM = mRootSpan->mWritingMode;
 
@@ -1451,32 +1461,16 @@ nsLineLayout::VerticalAlignLine()
   for (PerFrameData* pfd = psd->mFirstFrame; pfd; pfd = pfd->mNext) {
     if (pfd->mBlockDirAlign == VALIGN_OTHER) {
       pfd->mBounds.BStart(lineWM) += baselineBCoord;
       pfd->mFrame->SetRect(lineWM, pfd->mBounds, mContainerWidth);
     }
   }
   PlaceTopBottomFrames(psd, -mBStartEdge, lineBSize);
 
-  // If the frame being reflowed has text decorations, we simulate the
-  // propagation of those decorations to a line-level element by storing the
-  // offset in a frame property on any child frames that are aligned in the
-  // block direction somewhere other than the baseline. This property is then
-  // used by nsTextFrame::GetTextDecorations when the same conditions are met.
-  if (rootPFD.mFrame->StyleContext()->HasTextDecorationLines()) {
-    for (const PerFrameData* pfd = psd->mFirstFrame; pfd; pfd = pfd->mNext) {
-      const nsIFrame *const f = pfd->mFrame;
-      if (f->VerticalAlignEnum() != NS_STYLE_VERTICAL_ALIGN_BASELINE) {
-        const nscoord offset = baselineBCoord - pfd->mBounds.BStart(lineWM);
-        f->Properties().Set(nsIFrame::LineBaselineOffset(),
-                            NS_INT32_TO_PTR(offset));
-      }
-    }
-  }
-
   // Fill in returned line-box and max-element-width data
   mLineBox->SetBounds(lineWM,
                       psd->mIStart, mBStartEdge,
                       psd->mICoord - psd->mIStart, lineBSize,
                       mContainerWidth);
 
   mFinalLineBSize = lineBSize;
   mLineBox->SetLogicalAscent(baselineBCoord - mBStartEdge);
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -4615,16 +4615,50 @@ PaintSelectionBackground(gfxContext* aCt
   if (aCallbacks) {
     aCallbacks->NotifySelectionBackgroundPathEmitted();
   } else {
     aCtx->SetColor(gfxRGBA(aColor));
     aCtx->Fill();
   }
 }
 
+// Attempt to get the LineBaselineOffset property of aChildFrame
+// If not set, calculate this value for all child frames of aBlockFrame
+static nscoord
+LazyGetLineBaselineOffset(nsIFrame* aChildFrame, nsBlockFrame* aBlockFrame)
+{
+  bool offsetFound;
+  nscoord offset = NS_PTR_TO_INT32(
+    aChildFrame->Properties().Get(nsIFrame::LineBaselineOffset(), &offsetFound)
+    );
+
+  if (!offsetFound) {
+    for (nsBlockFrame::line_iterator line = aBlockFrame->begin_lines(),
+        line_end = aBlockFrame->end_lines();
+        line != line_end; line++) {
+      if (line->IsInline()) {
+        int32_t n = line->GetChildCount();
+        nscoord lineBaseline = line->BStart() + line->GetLogicalAscent();
+        for (nsIFrame* lineFrame = line->mFirstChild;
+             n > 0; lineFrame = lineFrame->GetNextSibling(), --n) {
+          offset = lineBaseline - lineFrame->GetNormalPosition().y;
+          lineFrame->Properties().Set(nsIFrame::LineBaselineOffset(),
+                                      NS_INT32_TO_PTR(offset));
+        }
+      }
+    }
+    return NS_PTR_TO_INT32(
+    aChildFrame->Properties().Get(nsIFrame::LineBaselineOffset(), &offsetFound)
+    );
+
+  } else {
+    return offset;
+  }
+}
+
 void
 nsTextFrame::GetTextDecorations(
                     nsPresContext* aPresContext,
                     nsTextFrame::TextDecorationColorResolution aColorResolution,
                     nsTextFrame::TextDecorations& aDecorations)
 {
   const nsCompatibility compatMode = aPresContext->CompatibilityMode();
 
@@ -4658,34 +4692,37 @@ nsTextFrame::GetTextDecorations(
         (NS_STYLE_TEXT_DECORATION_LINE_OVERRIDE_ALL & textDecorations)) {
       // This handles the <a href="blah.html"><font color="green">La 
       // la la</font></a> case. The link underline should be green.
       useOverride = true;
       overrideColor =
         nsLayoutUtils::GetColor(f, eCSSProperty_text_decoration_color);
     }
 
-    const bool firstBlock = !nearestBlockFound && nsLayoutUtils::GetAsBlock(f);
+    nsBlockFrame* fBlock = nsLayoutUtils::GetAsBlock(f);
+    const bool firstBlock = !nearestBlockFound && fBlock;
 
     // Not updating positions once we hit a parent block is equivalent to
     // the CSS 2.1 spec that blocks should propagate decorations down to their
     // children (albeit the style should be preserved)
     // However, if we're vertically aligned within a block, then we need to
     // recover the correct baseline from the line by querying the FrameProperty
     // that should be set (see nsLineLayout::VerticalAlignLine).
     if (firstBlock) {
       // At this point, fChild can't be null since TextFrames can't be blocks
       if (fChild->VerticalAlignEnum() != NS_STYLE_VERTICAL_ALIGN_BASELINE) {
+
         // Since offset is the offset in the child's coordinate space, we have
         // to undo the accumulation to bring the transform out of the block's
         // coordinate space
+        const nscoord lineBaselineOffset = LazyGetLineBaselineOffset(fChild,
+                                                                     fBlock);
+
         baselineOffset =
-          frameTopOffset - fChild->GetNormalPosition().y
-          - NS_PTR_TO_INT32(
-              fChild->Properties().Get(nsIFrame::LineBaselineOffset()));
+          frameTopOffset - fChild->GetNormalPosition().y - lineBaselineOffset;
       }
     }
     else if (!nearestBlockFound) {
       // use a dummy WritingMode, because nsTextFrame::GetLogicalBaseLine
       // doesn't use it anyway
       baselineOffset = frameTopOffset - f->GetLogicalBaseline(WritingMode());
     }
 
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-1-ref.html
@@ -0,0 +1,21 @@
+<html>
+<head>
+<style>
+.underline {
+  text-decoration: underline;
+}
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p class="underline">
+<span class="align-bottom">This</span> line has a bottom vertical align span. <br />
+<span class="align-top">This</span> line has a top vertical align span.
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-1.html
@@ -0,0 +1,26 @@
+<html class="reftest-wait">
+<head>
+<script type="text/javascript">
+function addUnderline() {
+  var element = document.getElementById("dynamicUnderline");
+  element.style.textDecoration = "underline";
+  document.documentElement.removeAttribute("class");
+}
+document.addEventListener('MozReftestInvalidate', addUnderline, false);
+</script>
+<style>
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p id="dynamicUnderline">
+<span class="align-bottom">This</span> line has a bottom vertical align span. <br />
+<span class="align-top">This</span> line has a top vertical align span.
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-2-ref.html
@@ -0,0 +1,21 @@
+<html>
+<head>
+<style>
+.underline {
+  text-decoration: underline;
+}
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p class="underline">
+<span class="align-bottom">This line has only a bottom vertical align span.</span> <br />
+<span class="align-top">This line has a top vertical align span.</span>
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-quirks-2.html
@@ -0,0 +1,26 @@
+<html class="reftest-wait">
+<head>
+<script type="text/javascript">
+function addUnderline() {
+  var element = document.getElementById("dynamicUnderline");
+  element.style.textDecoration = "underline";
+  document.documentElement.removeAttribute("class");
+}
+document.addEventListener('MozReftestInvalidate', addUnderline, false);
+</script>
+<style>
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p id="dynamicUnderline">
+<span class="align-bottom">This line has only a bottom vertical align span.</span> <br />
+<span class="align-top">This line has a top vertical align span.</span>
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-1-ref.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.underline {
+  text-decoration: underline;
+}
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p class="underline">
+<span class="align-bottom">This</span> line has a bottom vertical align span. <br />
+<span class="align-top">This</span> line has a top vertical align span.
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-1.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<script type="text/javascript">
+function addUnderline() {
+  var element = document.getElementById("dynamicUnderline");
+  element.style.textDecoration = "underline";
+  document.documentElement.removeAttribute("class");
+}
+document.addEventListener('MozReftestInvalidate', addUnderline, false);
+</script>
+<style>
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p id="dynamicUnderline">
+<span class="align-bottom">This</span> line has a bottom vertical align span. <br />
+<span class="align-top">This</span> line has a top vertical align span.
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-2-ref.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.underline {
+  text-decoration: underline;
+}
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p class="underline">
+<span class="align-bottom">This line has only a bottom vertical align span.</span> <br />
+<span class="align-top">This line has a top vertical align span.</span>
+</p>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text-decoration/dynamic-underline-vertical-align-standards-2.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<script type="text/javascript">
+function addUnderline() {
+  var element = document.getElementById("dynamicUnderline");
+  element.style.textDecoration = "underline";
+  document.documentElement.removeAttribute("class");
+}
+document.addEventListener('MozReftestInvalidate', addUnderline, false);
+</script>
+<style>
+.align-bottom {
+  vertical-align: bottom;
+}
+.align-top {
+  vertical-align: top;
+}
+</style>
+</head>
+<body>
+<p id="dynamicUnderline">
+<span class="align-bottom">This line has only a bottom vertical align span.</span> <br />
+<span class="align-top">This line has a top vertical align span.</span>
+</p>
+</body>
+</html>
--- a/layout/reftests/text-decoration/reftest.list
+++ b/layout/reftests/text-decoration/reftest.list
@@ -1,14 +1,18 @@
 skip-if(B2G) == complex-decoration-style-quirks.html complex-decoration-style-quirks-ref.html # bug 773482
 skip-if(B2G) == complex-decoration-style-standards.html complex-decoration-style-standards-ref.html # bug 773482
 == decoration-color-quirks.html decoration-color-quirks-ref.html
 == decoration-color-standards.html decoration-color-standards-ref.html
 == decoration-style-quirks.html decoration-style-quirks-ref.html
 == decoration-style-standards.html decoration-style-standards-ref.html
+fuzzy-if(B2G,255,1) == dynamic-underline-vertical-align-quirks-1.html dynamic-underline-vertical-align-quirks-1-ref.html
+fuzzy-if(B2G,255,1) == dynamic-underline-vertical-align-standards-1.html dynamic-underline-vertical-align-standards-1-ref.html
+fails == dynamic-underline-vertical-align-quirks-2.html dynamic-underline-vertical-align-quirks-2-ref.html
+fuzzy-if(B2G,255,1) == dynamic-underline-vertical-align-standards-2.html dynamic-underline-vertical-align-standards-2-ref.html
 == line-through-style-block-solid-quirks.html line-through-style-block-quirks-ref.html
 != line-through-style-block-dotted-quirks.html line-through-style-block-quirks-ref.html
 != line-through-style-block-dashed-quirks.html line-through-style-block-quirks-ref.html
 != line-through-style-block-double-quirks.html line-through-style-block-quirks-ref.html
 != line-through-style-block-wavy-quirks.html line-through-style-block-quirks-ref.html
 == line-through-style-inline-solid-quirks.html line-through-style-inline-quirks-ref.html
 != line-through-style-inline-dotted-quirks.html line-through-style-inline-quirks-ref.html
 != line-through-style-inline-dashed-quirks.html line-through-style-inline-quirks-ref.html