Bug 1426760: Don't let placeholders affect line height. r=jfkthame

MozReview-Commit-ID: IJHerDOKOkj

--HG--
extra : rebase_source : 054503061bcafad8f577bc1f59a12ec8797cc4d0

layout/generic/nsBlockFrame.cpp

@@ -4147,11 +4147,7 @@ nsBlockFrame::ReflowInlineFrame(BlockReflowInput& aState,
                                 nsIFrame* aFrame,
                                 LineReflowStatus* aLineReflowStatus)
 {
-  if (!aFrame) { // XXX change to MOZ_ASSERT(aFrame)
-    NS_ERROR("why call me?");
-    return;
-  }
-
+  MOZ_ASSERT(aFrame);
   *aLineReflowStatus = LineReflowStatus::OK;
 
 #ifdef NOISY_FIRST_LETTER
@@ -4169,7 +4165,7 @@ nsBlockFrame::ReflowInlineFrame(BlockReflowInput& aState,
 
   // Reflow the inline frame
   nsReflowStatus frameReflowStatus;
-  bool           pushedFrame;
+  bool pushedFrame;
   aLineLayout.ReflowFrame(aFrame, frameReflowStatus, nullptr, pushedFrame);
 
   if (frameReflowStatus.NextInFlowNeedsReflow()) {

layout/generic/nsLineLayout.cpp

@@ -662,6 +662,7 @@ nsLineLayout::NewPerFrameData(nsIFrame* aFrame)
   pfd->mIsBullet = false;
   pfd->mSkipWhenTrimmingWhitespace = false;
   pfd->mIsEmpty = false;
+  pfd->mIsPlaceholder = false;
   pfd->mIsLinkedToBase = false;
 
   pfd->mWritingMode = aFrame->GetWritingMode();
@@ -840,7 +841,7 @@ nsLineLayout::ReflowFrame(nsIFrame* aFrame,
 
   // Figure out whether we're talking about a textframe here
   LayoutFrameType frameType = aFrame->Type();
-  bool isText = frameType == LayoutFrameType::Text;
+  const bool isText = frameType == LayoutFrameType::Text;
 
   // Inline-ish and text-ish things don't compute their width;
   // everything else does.  We need to give them an available width that
@@ -940,6 +941,7 @@ nsLineLayout::ReflowFrame(nsIFrame* aFrame,
   } else {
     if (LayoutFrameType::Placeholder == frameType) {
       isEmpty = true;
+      pfd->mIsPlaceholder = true;
       pfd->mSkipWhenTrimmingWhitespace = true;
       nsIFrame* outOfFlowFrame = nsLayoutUtils::GetFloatFromPlaceholder(aFrame);
       if (outOfFlowFrame) {
@@ -2209,17 +2211,17 @@ nsLineLayout::VerticalAlignFrames(PerSpanData* psd)
       //           For example in quirks mode, avoiding empty text frames prevents
       //           "tall" lines around elements like <hr> since the rules of <hr>
       //           in quirks.css have pseudo text contents with LF in them.
-#if 0
-      if (!pfd->mIsTextFrame) {
-#else
-      // Only consider non empty text frames when line-height=normal
-      bool canUpdate = !pfd->mIsTextFrame;
-      if (!canUpdate && pfd->mIsNonWhitespaceTextFrame) {
-        canUpdate =
+      bool canUpdate;
+      if (pfd->mIsTextFrame) {
+        // Only consider text frames if they're not empty and
+        // line-height=normal.
+        canUpdate = pfd->mIsNonWhitespaceTextFrame &&
           frame->StyleText()->mLineHeight.GetUnit() == eStyleUnit_Normal;
+      } else {
+        canUpdate = !pfd->mIsPlaceholder;
       }
+
       if (canUpdate) {
-#endif
         nscoord blockStart, blockEnd;
         if (frameSpan) {
           // For spans that were are now placing, use their position

layout/generic/nsLineLayout.h

@@ -471,6 +471,7 @@ protected:
     bool mIsBullet : 1;
     bool mSkipWhenTrimmingWhitespace : 1;
     bool mIsEmpty : 1;
+    bool mIsPlaceholder : 1;
     bool mIsLinkedToBase : 1;
 
     // Other state we use

testing/web-platform/meta/MANIFEST.json

@@ -52773,6 +52773,18 @@
      {}
     ]
    ],
+   "css/CSS2/linebox/line-height-oof-descendants-001.html": [
+    [
+     "/css/CSS2/linebox/line-height-oof-descendants-001.html",
+     [
+      [
+       "/css/CSS2/linebox/line-height-oof-descendants-001-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/CSS2/linebox/vertical-align-004.xht": [
     [
      "/css/CSS2/linebox/vertical-align-004.xht",
@@ -215920,6 +215932,11 @@
      {}
     ]
    ],
+   "css/CSS2/linebox/line-height-oof-descendants-001-ref.html": [
+    [
+     {}
+    ]
+   ],
    "css/CSS2/linebox/support/1x1-green.png": [
     [
      {}
@@ -429253,6 +429270,14 @@
    "2927ed2a8c86f6a791db5d6eb670ad1961b17e9e",
    "visual"
   ],
+  "css/CSS2/linebox/line-height-oof-descendants-001-ref.html": [
+   "284fd0f610f5428bea7a5f9c0dee1bdde3a4670b",
+   "support"
+  ],
+  "css/CSS2/linebox/line-height-oof-descendants-001.html": [
+   "bb8949f890f140305ac76beb3f3ae1f2d15b16a3",
+   "reftest"
+  ],
   "css/CSS2/linebox/support/1x1-green.png": [
    "51e7b6974a09eda6cb31337717c5eaeb9c44b443",
    "support"
@@ -545838,7 +545863,7 @@
    "testharness"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-1.html": [
-   "2b4fa3b558dccb50bf0aee12a78e3320501ea1b5",
+   "b48335aa61dc13c34d2a77806f20663e2156bc6f",
    "testharness"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-1.js": [
@@ -545846,15 +545871,15 @@
    "support"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html": [
-   "70271ef6fbf9f6e4f6e61438691b6fce317137e9",
+   "e2c860b1b348148fc6b9d77f918894b1bac42c94",
    "testharness"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html": [
-   "80fa90a214bb4839703c36f9db36e07f3a2ca7f2",
+   "996d1aa45c5975e13ac0f1e9c9249b3d452ed2e2",
    "testharness"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html": [
-   "dbb8eb640576cd4f658e32dec441919e943f8d21",
+   "224fe5510f09c3dd6d58f9dcf61b4d6fca04c96c",
    "testharness"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-4a.js": [
@@ -545874,7 +545899,7 @@
    "support"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html": [
-   "fcc8da57e88ee87592a02888c54bb6d66e5172f6",
+   "7239ae9f5705f7baf5630e67cf4bfdc6c25b108d",
    "testharness"
   ],
   "html/semantics/scripting-1/the-script-element/module/instantiation-error-5a.js": [

testing/web-platform/tests/css/CSS2/linebox/line-height-oof-descendants-001-ref.html

@@ -0,0 +1,14 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<style>
+p {
+  font-size: 20px;
+  line-height: 0;
+}
+</style>
+<p>Some paragraph</p>
+<p>Some paragraph</p>
+<p>Some paragraph</p>
+<p>Some other paragraph</p>

testing/web-platform/tests/css/CSS2/linebox/line-height-oof-descendants-001.html

@@ -0,0 +1,17 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: line-height is not affected by out-of-flow descendants</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://www.w3.org/TR/CSS21/visudet.html#line-height">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1426760">
+<link rel="match" href="line-height-oof-descendants-001-ref.html">
+<style>
+p {
+  font-size: 20px;
+  line-height: 0;
+}
+</style>
+<p><span style="position: absolute;"></span>Some paragraph</p>
+<p><span style="float: left;"></span>Some paragraph</p>
+<p><span style="position: fixed;"></span>Some paragraph</p>
+<p>Some other paragraph</p>