Bug 1716212 Part 2 - Carry nsComboboxControlFrame's line-height to nsComboboxDisplayFrame when it has auto block-size. r=emilio

The issue is that combobox display may contain some non-Latin characters that need extra block-size to display than the one line-height calculate by using a Latin font spec in combobox control's style. Before this patch, when a combobox control has unconstrained block-size, we set combobox display's block-size to combobox control's one line-height in Reflow(), which is intended to properly initialize `BlockReflowInput::mMinLineHeight` since combobox display has `line-height:-moz-block-height`. However, this simply prevents the combobox display from choosing a larger block-size after the reflow. See bug 1716212 comment 11 for an analysis. This patch fixes the issue by carrying combox control's computed line height to combobox display so that its computed block-size is still unconstrained so that it can accommodate taller characters in the display text. After this patch, <select><option> containing non-Latin characters should have the same block-size as <button>, and no characters should be clipped. Modified test_unstyled_control_height.html to test this. Differential Revision: https://phabricator.services.mozilla.com/D120877
2021-07-28 18:41:19 +00:00 · 2021-07-28 18:41:19 +00:00 · 8b74ff00e0
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@ -1245,18 +1245,13 @@ void nsComboboxDisplayFrame::Reflow(nsPresContext* aPresContext,
                                    const ReflowInput& aReflowInput,
                                    nsReflowStatus& aStatus) {
  MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!");
+  MOZ_ASSERT(aReflowInput.mParentReflowInput &&
+                 aReflowInput.mParentReflowInput->mFrame == mComboBox,
+             "Combobox's frame tree is wrong!");

  ReflowInput state(aReflowInput);
  if (state.ComputedBSize() == NS_UNCONSTRAINEDSIZE) {
-    float inflation = nsLayoutUtils::FontSizeInflationFor(mComboBox);
-    // We intentionally use the combobox frame's style here, which has
-    // the 'line-height' specified by the author, if any.
-    // (This frame has 'line-height: -moz-block-height' in the UA
-    // sheet which is suitable when there's a specified block-size.)
-    nscoord lh = ReflowInput::CalcLineHeight(mComboBox->GetContent(),
-                                             mComboBox->Style(), aPresContext,
-                                             NS_UNCONSTRAINEDSIZE, inflation);
-    state.SetComputedBSize(lh);
+    state.SetLineHeight(state.mParentReflowInput->GetLineHeight());
  }
  const WritingMode wm = aReflowInput.GetWritingMode();
  const LogicalMargin bp = state.ComputedLogicalBorderPadding(wm);
--- a/layout/forms/test/test_unstyled_control_height.html
+++ b/layout/forms/test/test_unstyled_control_height.html
@ -1,10 +1,15 @@
 <!doctype html>
+<meta charset="utf-8">
 <script src="/tests/SimpleTest/SimpleTest.js"></script>
 <style>
-  #container * {
+  #container *, #container2 * {
    white-space: nowrap;
    appearance: none;
  }
+  input {
+    /* Reduce the width so that container can fit all its children in 600px viewport width. */
+    width: 100px;
+  }
  input[type=date] {
    /* date by default uses a monospace font, which might have different metrics */
    font: -moz-field;
@ -16,17 +21,31 @@
    padding: 0;
  }
 </style>
+
+<!-- Each container should fit all its children in the same line to verify every
+     child has the same |top|. -->
 <div id="container">
  <input>
+  <!-- Putting the <input> containing Burmese characters here is just to verify
+       our current behavior. They are slightly clipped. So if we fix it by
+       making the <input> taller, it's OK to remove it from this test. -->
+  <input value="漢字 jpg မြန်မာစာ">
  <input type=date>
  <button>Foo</button>
  <select><option>Foo</option></select>
 </div>
+
+<br>
+<div id="container2">
+  <button>漢字 Foo မြန်မာစာ</button>
+  <select><option>漢字 Foo မြန်မာစာ</option></select>
+</div>
+
 <script>
-function testHeightMatches(desc) {
+function testHeightMatches(id, desc) {
  let commonHeight = null;
  let commonTop = null;
-  for (let element of document.querySelectorAll("#container > *")) {
+  for (let element of document.querySelectorAll(`#${id} > *`)) {
    let rect = element.getBoundingClientRect();
    if (commonHeight === null) {
      commonHeight = rect.height;
@ -37,15 +56,17 @@ function testHeightMatches(desc) {
  }
 }

-const container = document.getElementById("container");
+for (id of ["container", "container2"]) {
+  const container = document.getElementById(id);

-testHeightMatches("");
+  testHeightMatches(id, "");

  container.className = "no-padding";

-testHeightMatches(" without padding");
+  testHeightMatches(id, " without padding");

  container.className = "small-font";

-testHeightMatches(" with an small font");
+  testHeightMatches(id, " with an small font");
+}
 </script>
--- a/layout/generic/ReflowInput.cpp
+++ b/layout/generic/ReflowInput.cpp
@ -2721,6 +2721,17 @@ nscoord ReflowInput::GetLineHeight() const {
  return mLineHeight;
 }

+void ReflowInput::SetLineHeight(nscoord aLineHeight) {
+  MOZ_ASSERT(aLineHeight >= 0, "aLineHeight must be >= 0!");
+
+  if (mLineHeight != aLineHeight) {
+    mLineHeight = aLineHeight;
+    // Setting used line height can change a frame's block-size if mFrame's
+    // block-size behaves as auto.
+    InitResizeFlags(mFrame->PresContext(), mFrame->Type());
+  }
+}
+
 /* static */
 nscoord ReflowInput::CalcLineHeight(nsIContent* aContent,
                                    ComputedStyle* aComputedStyle,
--- a/layout/generic/ReflowInput.h
+++ b/layout/generic/ReflowInput.h
@ -697,6 +697,11 @@ struct ReflowInput : public SizeComputationInput {
   */
  nscoord GetLineHeight() const;

+  /**
+   * Set the used line-height. aLineHeight must be >= 0.
+   */
+  void SetLineHeight(nscoord aLineHeight);
+
  /**
   * Calculate the used line-height property without a reflow input instance.
   * The return value will be >= 0.