Bug 1499578 - Make one line-height the intrinsic block-size for <select> comboboxes. r=emilio
authorMats Palmgren <mats@mozilla.com>
Mon, 22 Oct 2018 00:54:56 +0200
changeset 490612 50a3c9a567e3f8f6285ba6bb0c0b7e5e6a4358e9
parent 490611 b2fa4b07f6f7e489047808bd0238301ea943b4b3
child 490613 a9b1d69e379ac93067c87570fd0b572d3af4c22c
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersemilio
bugs1499578
milestone64.0a1
Bug 1499578 - Make one line-height the intrinsic block-size for <select> comboboxes. r=emilio Also, remove the !important on 'line-height' on <select> since other UAs allows authors to change it.
layout/forms/nsComboboxControlFrame.cpp
layout/forms/test/test_bug348236.html
layout/style/res/forms.css
layout/style/test/test_animations.html
testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-ref.html
testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size.html
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1333,29 +1333,36 @@ public:
 
 protected:
   nsComboboxControlFrame* mComboBox;
 };
 
 NS_IMPL_FRAMEARENA_HELPERS(nsComboboxDisplayFrame)
 
 void
-nsComboboxDisplayFrame::Reflow(nsPresContext*           aPresContext,
-                               ReflowOutput&     aDesiredSize,
+nsComboboxDisplayFrame::Reflow(nsPresContext*     aPresContext,
+                               ReflowOutput&      aDesiredSize,
                                const ReflowInput& aReflowInput,
-                               nsReflowStatus&          aStatus)
+                               nsReflowStatus&    aStatus)
 {
   MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!");
 
   ReflowInput state(aReflowInput);
   if (state.ComputedBSize() == NS_INTRINSICSIZE) {
-    // Note that the only way we can have a computed block size here is
-    // if the combobox had a specified block size.  If it didn't, size
-    // based on what our rows look like, for lack of anything better.
-    state.SetComputedBSize(mComboBox->mListControlFrame->GetBSizeOfARow());
+    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.)
+    auto lh = ReflowInput::CalcLineHeight(mComboBox->GetContent(),
+                                          mComboBox->Style(),
+                                          aPresContext,
+                                          NS_UNCONSTRAINEDSIZE,
+                                          inflation);
+    state.SetComputedBSize(lh);
   }
   WritingMode wm = aReflowInput.GetWritingMode();
   nscoord computedISize = mComboBox->mDisplayISize -
     state.ComputedLogicalBorderPadding().IStartEnd(wm);
   if (computedISize < 0) {
     computedISize = 0;
   }
   state.SetComputedISize(computedISize);
--- a/layout/forms/test/test_bug348236.html
+++ b/layout/forms/test/test_bug348236.html
@@ -21,17 +21,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=348236">Mozilla Bug 348236</a>
 <p id="display"></p>
 <div id="content">
 
   <select id="eSelect" size="1" onchange="++this.onchangeCount">
     <option selected>1</option>
     <option>2</option>
-    <option>3</option>
+    <option id="option3">3</option>
   </select>
 </div>
 <pre id="test">
 <script type="text/javascript">
 
   /** Test for Bug 348236 **/
 
 SimpleTest.waitForExplicitFinish()
@@ -54,18 +54,20 @@ addLoadEvent(function test() {
         eSelect.selectedIndex = 0
         eSelect.onchangeCount = 0
 
         // Drop the SELECT down.
         keypressOnSelect(key)
         // This timeout and the following are necessary to let the sent events take effect.
         setTimeout(cont1, timeout)
         function cont1() {
-            // Move the mouse over option 3 (90 = 3 (rows) * 24 (row height) + 18).
-            WinUtils.sendMouseEvent("mousemove", 355, 92, 0, 0, 0, true)
+            // Move the mouse over option 3.
+            let option3 = document.getElementById("option3");
+            let rect = option3.getBoundingClientRect();
+            WinUtils.sendMouseEvent("mousemove", rect.left + 4, rect.top + 4, 0, 0, 0, true)
             setTimeout(cont2, timeout)
         }
         function cont2() {
             // Close the select.
             keypressOnSelect(key)
             setTimeout(cont3, timeout)
         }
         function cont3() {
--- a/layout/style/res/forms.css
+++ b/layout/style/res/forms.css
@@ -220,23 +220,17 @@ textarea:-moz-read-write {
 }
 
 select {
   margin: 0;
   border-color: ThreeDLightShadow;
   background-color: -moz-Combobox;
   color: -moz-ComboboxText;
   font: -moz-list;
-  /*
-   * Note that the "UA !important" tests in
-   * layout/style/test/test_animations.html depend on this rule, because
-   * they need some UA !important rule to test.  If this changes, use a
-   * different one there.
-   */
-  line-height: normal !important;
+  line-height: initial;
   white-space: nowrap !important;
   word-wrap: normal !important;
   text-align: start;
   cursor: default;
   box-sizing: border-box;
   -moz-user-select: none;
   -moz-appearance: menulist;
   border-width: 2px;
@@ -340,16 +334,22 @@ select:empty {
   line-height: -moz-block-height;
 }
 
 option {
   display: block;
   float: none !important;
   position: static !important;
   min-block-size: 1em;
+  /*
+   * Note that the "UA !important" tests in
+   * layout/style/test/test_animations.html depend on this rule, because
+   * they need some UA !important rule to test.  If this changes, use a
+   * different one there.
+   */
   line-height: normal !important;
   -moz-user-select: none;
   text-indent: 0;
   white-space: nowrap !important;
   word-wrap: normal !important;
   text-align: match-parent;
 }
 
--- a/layout/style/test/test_animations.html
+++ b/layout/style/test/test_animations.html
@@ -1374,23 +1374,23 @@ is(cs.marginRight, "10px", "bug 651456 a
 advance_clock(0); // still forces a refresh
 is(cs.marginRight, "10px", "bug 651456 at 100ms (2)");
 advance_clock(100);
 is(cs.marginRight, "20px", "bug 651456 at 200ms");
 done_div();
 
 // Test that UA !important rules override animations.
 // This test depends on forms.css having a rule
-//   select { line-height: !important }
+//   option { line-height: !important }
 // If that rule changes, we should rewrite it to depend on a different rule.
-var select;
-[ select, cs ] = new_element("select", "");
+var option;
+[ option, cs ] = new_element("option", "");
 var default_line_height = cs.lineHeight;
 done_element();
-[ select, cs ] = new_element("select",
+[ option, cs ] = new_element("option",
                              "animation: uaoverride 2s linear infinite");
 is(cs.lineHeight, default_line_height,
    "animations should not override UA !important at 0ms");
 is(cs.marginTop, "20px",
    "rest of animation should still work when UA !important present at 0ms");
 advance_clock(200);
 is(cs.lineHeight, default_line_height,
    "animations should not override UA !important at 200ms");
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-ref.html
@@ -0,0 +1,33 @@
+<!DOCTYPE HTML>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html><head>
+  <meta charset="utf-8">
+  <title>Reference: Combobox block-size test</title>
+  <link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+  <style>
+html,body {
+  color:black; background-color:white; font:16px/1 monospace; padding:0; margin:0;
+}
+
+select { -webkit-appearance: none; }
+
+.big { font-size: 48pt;  min-height: 40pt; }
+.lh { line-height: 48pt; min-height: 40pt; }
+
+.mask { position:fixed; left:20px; right:0; top:0; bottom:0; background: black; }
+  </style>
+</head>
+<body>
+
+<!-- mask off differences on the right side -->
+<div class="mask"></div>
+
+<select><optgroup label="label"><option>option</option></select><br>
+<select class="big"><optgroup label="label"><option>option</option></select><br>
+<select class="lh"><optgroup label="label"><option>option</option></select><br>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size.html
@@ -0,0 +1,38 @@
+<!DOCTYPE HTML>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html><head>
+  <meta charset="utf-8">
+  <title>Test: Combobox block-size test</title>
+  <link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+  <link rel="match" href="select-1-block-size-ref.html">
+  <link rel="help" href="https://html.spec.whatwg.org/multipage/rendering.html#the-select-element-2">
+  <link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1499578">
+  <style>
+html,body {
+  color:black; background-color:white; font:16px/1 monospace; padding:0; margin:0;
+}
+
+select { -webkit-appearance: none; }
+
+optgroup { font-size: 32pt; }
+option { font-size: 24pt; }
+.big { font-size: 48pt; }
+.lh { line-height: 48pt; }
+
+.mask { position:fixed; left:20px; right:0; top:0; bottom:0; background: black; }
+  </style>
+</head>
+<body>
+
+<!-- mask off differences on the right side -->
+<div class="mask"></div>
+
+<select><optgroup label="label"><option>option</option></select><br>
+<select class="big"><optgroup label="label"><option>option</option></select><br>
+<select class="lh"><optgroup label="label"><option>option</option></select><br>
+
+</body>
+</html>