Bug 1932800 - Fix caret baseline of empty inline and text frames. r=dshin

Added Gecko reftests because I don't know what the best place to put
them in WPT is, and caret rendering is kind of undefined, but...

Differential Revision: https://phabricator.services.mozilla.com/D229926

layout/base/tests/marionette/test_accessiblecaret_cursor_mode.py

@@ -224,8 +224,8 @@ class AccessibleCaretCursorModeTestCase(MarionetteTestCase):
         self.open_test_html("layout/test_carets_columns.html")
         el = self.marionette.find_element(By.ID, "columns-inner")
         sel = SelectionManager(el)
+        original_content = sel.content
         content_to_add = "!"
-        target_content = sel.content + content_to_add
 
         # Goal: the cursor position can be changed by dragging the caret from
         # the front to the end of the content.
@@ -244,7 +244,16 @@ class AccessibleCaretCursorModeTestCase(MarionetteTestCase):
         self.actions.flick(el, src_x, src_y, dest_x, dest_y).perform()
 
         self.actions.send_keys(content_to_add).perform()
-        self.assertEqual(target_content, sel.content)
+        # Depending on where the column break ends up we might see an empty
+        # block, which adds a newline to the selection serialization.
+        # That's not related to the point of the test-case.
+        self.assertIn(
+            sel.content,
+            [
+                original_content + content_to_add,
+                original_content + "\n" + content_to_add,
+            ],
+        )
 
     def test_move_cursor_to_front_by_dragging_caret_to_front_br_element(self):
         self.open_test_html(self._cursor_html)

layout/generic/nsBlockFrame.cpp

@@ -661,28 +661,18 @@ Maybe<nscoord> nsBlockFrame::GetNaturalBaselineBOffset(
 }
 
 nscoord nsBlockFrame::GetCaretBaseline() const {
-  nsRect contentRect = GetContentRect();
-  nsMargin bp = GetUsedBorderAndPadding();
-
+  const auto wm = GetWritingMode();
   if (!mLines.empty()) {
     ConstLineIterator line = LinesBegin();
     if (!line->IsEmpty()) {
       if (line->IsBlock()) {
-        return bp.top + line->mFirstChild->GetCaretBaseline();
+        return GetLogicalUsedBorderAndPadding(wm).BStart(wm) +
+               line->mFirstChild->GetCaretBaseline();
       }
       return line->BStart() + line->GetLogicalAscent();
     }
   }
-
-  float inflation = nsLayoutUtils::FontSizeInflationFor(this);
-  RefPtr<nsFontMetrics> fm =
-      nsLayoutUtils::GetFontMetricsForFrame(this, inflation);
-  nscoord lineHeight = ReflowInput::CalcLineHeight(
-      *Style(), PresContext(), GetContent(), contentRect.height, inflation);
-  const WritingMode wm = GetWritingMode();
-  return nsLayoutUtils::GetCenteredFontBaseline(fm, lineHeight,
-                                                wm.IsLineInverted()) +
-         bp.top;
+  return GetFontMetricsDerivedCaretBaseline(ContentBSize(wm));
 }
 
 /////////////////////////////////////////////////////////////////////////////
@@ -2997,8 +2987,8 @@ static void DumpLine(const BlockReflowState& aState, nsLineBox* aLine,
 #endif
 }
 
-static bool LinesAreEmpty(const nsLineList& aList) {
-  for (const auto& line : aList) {
+bool nsBlockFrame::LinesAreEmpty() const {
+  for (const auto& line : mLines) {
     if (!line.IsEmpty()) {
       return false;
     }
@@ -3718,7 +3708,7 @@ bool nsBlockFrame::ReflowDirtyLines(BlockReflowState& aState) {
     }
   }
 
-  if (LinesAreEmpty(mLines) && ShouldHaveLineIfEmpty()) {
+  if (LinesAreEmpty() && ShouldHaveLineIfEmpty()) {
     aState.mBCoord += aState.mMinLineHeight;
   }
 
@@ -4103,7 +4093,7 @@ bool nsBlockFrame::IsEmpty() {
     return false;
   }
 
-  return LinesAreEmpty(mLines);
+  return LinesAreEmpty();
 }
 
 bool nsBlockFrame::ShouldApplyBStartMargin(BlockReflowState& aState,

layout/generic/nsBlockFrame.h

@@ -230,6 +230,7 @@ class nsBlockFrame : public nsContainerFrame {
   bool IsEmpty() override;
   bool CachedIsEmpty() override;
   bool IsSelfEmpty() override;
+  bool LinesAreEmpty() const;
 
   // Given that we have a ::marker frame, does it actually draw something, i.e.,
   // do we have either a 'list-style-type' or 'list-style-image' that is

layout/generic/nsIFrame.cpp

@@ -325,6 +325,16 @@ bool nsIFrame::CheckAndClearPaintedState() {
   return result;
 }
 
+nsIFrame* nsIFrame::FindLineContainer() const {
+  MOZ_ASSERT(IsLineParticipant());
+  nsIFrame* parent = GetParent();
+  while (parent &&
+         (parent->IsLineParticipant() || parent->CanContinueTextRun())) {
+    parent = parent->GetParent();
+  }
+  return parent;
+}
+
 bool nsIFrame::CheckAndClearDisplayListState() {
   bool result = BuiltDisplayList();
   SetBuiltDisplayList(false);
@@ -2134,6 +2144,18 @@ nsIFrame::CaretBlockAxisMetrics nsIFrame::GetCaretBlockAxisMetrics(
   return CaretBlockAxisMetrics{.mOffset = baseline - ascent, .mExtent = height};
 }
 
+nscoord nsIFrame::GetFontMetricsDerivedCaretBaseline(nscoord aBSize) const {
+  float inflation = nsLayoutUtils::FontSizeInflationFor(this);
+  RefPtr<nsFontMetrics> fm =
+      nsLayoutUtils::GetFontMetricsForFrame(this, inflation);
+  const WritingMode wm = GetWritingMode();
+  nscoord lineHeight = ReflowInput::CalcLineHeight(
+      *Style(), PresContext(), GetContent(), aBSize, inflation);
+  return nsLayoutUtils::GetCenteredFontBaseline(fm, lineHeight,
+                                                wm.IsLineInverted()) +
+         GetLogicalUsedBorderAndPadding(wm).BStart(wm);
+}
+
 const nsAtom* nsIFrame::ComputePageValue(const nsAtom* aAutoValue) const {
   const nsAtom* value = aAutoValue ? aAutoValue : nsGkAtoms::_empty;
   const nsIFrame* frame = this;

layout/generic/nsIFrame.h

@@ -1798,6 +1798,17 @@ class nsIFrame : public nsQueryFrame {
     return GetLogicalBaseline(GetWritingMode());
   }
 
+  // Gets a caret baseline suitable for the frame if the frame doesn't have one.
+  //
+  // @param aBSize the content box block size of the line container. Needed to
+  // resolve line-height: -moz-block-height. NS_UNCONSTRAINEDSIZE is fine
+  // otherwise.
+  //
+  // TODO(emilio): Now we support align-content on blocks it seems we could
+  // get rid of line-height: -moz-block-height.
+  nscoord GetFontMetricsDerivedCaretBaseline(
+      nscoord aBSize = NS_UNCONSTRAINEDSIZE) const;
+
   /**
    * Subclasses can call this method to enable visibility tracking for this
    * frame.
@@ -2612,6 +2623,11 @@ class nsIFrame : public nsQueryFrame {
    */
   virtual nsIFrame* LastInFlow() const { return const_cast<nsIFrame*>(this); }
 
+  /**
+   * Useful for line participants. Find the line container frame for this line.
+   */
+  nsIFrame* FindLineContainer() const;
+
   /**
    * Mark any stored intrinsic inline size information as dirty (requiring
    * re-calculation).  Note that this should generally not be called

layout/generic/nsInlineFrame.cpp

@@ -154,6 +154,21 @@ bool nsInlineFrame::IsEmpty() {
   return true;
 }
 
+nscoord nsInlineFrame::GetCaretBaseline() const {
+  if (mBaseline == 0 && mRect.IsEmpty()) {
+    nsBlockFrame* container = do_QueryFrame(FindLineContainer());
+    // TODO(emilio): Ideally we'd want to find out if only our line is empty,
+    // but that's non-trivial to do, and realistically empty inlines and text
+    // will get placed into a non-empty line unless all lines are empty, I
+    // believe...
+    if (container && container->LinesAreEmpty()) {
+      nscoord blockSize = container->ContentBSize(GetWritingMode());
+      return GetFontMetricsDerivedCaretBaseline(blockSize);
+    }
+  }
+  return nsIFrame::GetCaretBaseline();
+}
+
 nsIFrame::FrameSearchResult nsInlineFrame::PeekOffsetCharacter(
     bool aForward, int32_t* aOffset, PeekOffsetCharacterOptions aOptions) {
   // Override the implementation in nsFrame, to skip empty inline frames

layout/generic/nsInlineFrame.h

@@ -52,6 +52,8 @@ class nsInlineFrame : public nsContainerFrame {
   bool IsEmpty() override;
   bool IsSelfEmpty() override;
 
+  nscoord GetCaretBaseline() const override;
+
   FrameSearchResult PeekOffsetCharacter(
       bool aForward, int32_t* aOffset,
       PeekOffsetCharacterOptions aOptions =

layout/generic/nsTextFrame.cpp

@@ -1140,7 +1140,7 @@ class BuildTextRunsScanner {
   uint8_t mCurrentRunContextInfo;
 };
 
-static nsIFrame* FindLineContainer(nsIFrame* aFrame) {
+static const nsIFrame* FindLineContainer(const nsIFrame* aFrame) {
   while (aFrame &&
          (aFrame->IsLineParticipant() || aFrame->CanContinueTextRun())) {
     aFrame = aFrame->GetParent();
@@ -1148,6 +1148,11 @@ static nsIFrame* FindLineContainer(nsIFrame* aFrame) {
   return aFrame;
 }
 
+static nsIFrame* FindLineContainer(nsIFrame* aFrame) {
+  return const_cast<nsIFrame*>(
+      FindLineContainer(const_cast<const nsIFrame*>(aFrame)));
+}
+
 static bool IsLineBreakingWhiteSpace(char16_t aChar) {
   // 0x0A (\n) is not handled as white-space by the line breaker, since
   // we break before it, if it isn't transformed to a normal space.
@@ -8915,7 +8920,7 @@ void nsTextFrame::AddInlineMinISize(const IntrinsicSizeInput& aInput,
     if (f == this || f->GetTextRun(trtype) != lastTextRun) {
       nsIFrame* lc;
       if (aData->LineContainer() &&
-          aData->LineContainer() != (lc = FindLineContainer(f))) {
+          aData->LineContainer() != (lc = f->FindLineContainer())) {
         NS_ASSERTION(f != this,
                      "wrong InlineMinISizeData container"
                      " for first continuation");
@@ -9075,7 +9080,7 @@ void nsTextFrame::AddInlinePrefISize(const IntrinsicSizeInput& aInput,
     if (f == this || f->GetTextRun(trtype) != lastTextRun) {
       nsIFrame* lc;
       if (aData->LineContainer() &&
-          aData->LineContainer() != (lc = FindLineContainer(f))) {
+          aData->LineContainer() != (lc = f->FindLineContainer())) {
         NS_ASSERTION(f != this,
                      "wrong InlinePrefISizeData container"
                      " for first continuation");
@@ -10354,7 +10359,7 @@ bool nsTextFrame::AppendRenderedText(AppendRenderedTextState& aState,
   bool startsAtHardBreak, endsAtHardBreak;
   if (!HasAnyStateBits(TEXT_START_OF_LINE | TEXT_END_OF_LINE)) {
     startsAtHardBreak = endsAtHardBreak = false;
-  } else if (nsBlockFrame* thisLc = do_QueryFrame(FindLineContainer(this))) {
+  } else if (nsBlockFrame* thisLc = do_QueryFrame(FindLineContainer())) {
     if (thisLc != aState.mLineContainer) {
       // Setup line cursor when needed.
       aState.mLineContainer = thisLc;
@@ -10536,11 +10541,8 @@ bool nsTextFrame::IsEmpty() {
   if (textStyle->WhiteSpaceIsSignificant()) {
     // When WhiteSpaceIsSignificant styles are in effect, we only treat the
     // frame as empty if its content really is entirely *empty* (not just
-    // whitespace), AND it is NOT editable or within an <input> element.
-    // In these cases we consider that the whitespace-preserving style makes
-    // the frame behave as non-empty so that its height doesn't become zero.
-    return GetContentLength() == 0 && !GetContent()->IsEditable() &&
-           !GetContent()->GetParent()->IsHTMLElement(nsGkAtoms::input);
+    // whitespace).
+    return !GetContentLength();
   }
 
   if (HasAnyStateBits(TEXT_ISNOT_ONLY_WHITESPACE)) {
@@ -10696,6 +10698,21 @@ Maybe<nscoord> nsTextFrame::GetNaturalBaselineBOffset(
   return Some(parentAscent - (aWM.IsVertical() ? position.x : position.y));
 }
 
+nscoord nsTextFrame::GetCaretBaseline() const {
+  if (mAscent == 0 && HasAnyStateBits(TEXT_NO_RENDERED_GLYPHS)) {
+    nsBlockFrame* container = do_QueryFrame(FindLineContainer());
+    // TODO(emilio): Ideally we'd want to find out if only our line is empty,
+    // but that's non-trivial to do, and realistically empty inlines and text
+    // will get placed into a non-empty line unless all lines are empty, I
+    // believe...
+    if (container && container->LinesAreEmpty()) {
+      nscoord blockSize = container->ContentBSize(GetWritingMode());
+      return GetFontMetricsDerivedCaretBaseline(blockSize);
+    }
+  }
+  return nsIFrame::GetCaretBaseline();
+}
+
 bool nsTextFrame::HasAnyNoncollapsedCharacters() {
   gfxSkipCharsIterator iter = EnsureTextRun(nsTextFrame::eInflated);
   int32_t offset = GetContentOffset(), offsetEnd = GetContentEnd();

layout/generic/nsTextFrame.h

@@ -365,6 +365,7 @@ class nsTextFrame : public nsIFrame {
   Maybe<nscoord> GetNaturalBaselineBOffset(
       mozilla::WritingMode aWM, BaselineSharingGroup aBaselineGroup,
       BaselineExportContext) const override;
+  nscoord GetCaretBaseline() const override;
 
   bool HasSignificantTerminalNewline() const final;
 

layout/reftests/css-ui/caret-empty-contenteditable-01.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<style>
+  div, input {
+    margin: 0;
+    padding: 0;
+    border: 0;
+    color: transparent;
+    background-color: transparent;
+    caret-color: black;
+    outline: none;
+  }
+</style>
+<div contenteditable autofocus></div>

layout/reftests/css-ui/caret-empty-contenteditable-02.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<style>
+  div, input {
+    margin: 0;
+    padding: 0;
+    border: 0;
+    color: transparent;
+    background-color: transparent;
+    caret-color: black;
+    outline: none;
+  }
+</style>
+<div contenteditable autofocus><span></span></div>

layout/reftests/css-ui/caret-empty-contenteditable-03.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<style>
+  div, input {
+    margin: 0;
+    padding: 0;
+    border: 0;
+    color: transparent;
+    background-color: transparent;
+    caret-color: black;
+    outline: none;
+  }
+</style>
+<div contenteditable autofocus><span></span><br></div>

layout/reftests/css-ui/caret-empty-contenteditable-ref.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<style>
+  div, input {
+    margin: 0;
+    padding: 0;
+    border: 0;
+    color: transparent;
+    background-color: transparent;
+    caret-color: black;
+    outline: none;
+  }
+</style>
+<div contenteditable autofocus>x</div>

layout/reftests/css-ui/caret-empty-input-01.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<style>
+  div, input {
+    margin: 0;
+    padding: 0;
+    border: 0;
+    color: transparent;
+    background-color: transparent;
+    caret-color: black;
+    outline: none;
+  }
+</style>
+<input autofocus>

layout/reftests/css-ui/caret-empty-input-ref.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<style>
+  div, input {
+    margin: 0;
+    padding: 0;
+    border: 0;
+    color: transparent;
+    background-color: transparent;
+    caret-color: black;
+    outline: none;
+  }
+</style>
+<input autofocus value=x>

layout/reftests/css-ui/reftest.list

@@ -1 +1,13 @@
-fails-if(useDrawSnapshot) pref(ui.caretWidth,16) needs-focus == caret-color-01.html caret-color-01-ref.html
+defaults pref(ui.caretWidth,16)
+
+# drawSnapshot doesn't draw carets
+fails-if(useDrawSnapshot) needs-focus == caret-color-01.html caret-color-01-ref.html
+
+fails-if(useDrawSnapshot) needs-focus != caret-empty-contenteditable-ref.html about:blank
+
+needs-focus == caret-empty-contenteditable-01.html caret-empty-contenteditable-ref.html
+random needs-focus == caret-empty-contenteditable-02.html caret-empty-contenteditable-ref.html # Dependency on font metrics that needs to be sorted out.
+needs-focus == caret-empty-contenteditable-03.html caret-empty-contenteditable-ref.html
+
+fails-if(useDrawSnapshot) needs-focus != caret-empty-input-ref.html about:blank
+needs-focus == caret-empty-input-01.html caret-empty-input-ref.html