Bug 1571764 - Subtract combobox display frame border-padding when inferring the height from line-height. r=mats,jfkthame
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 15 Aug 2019 09:41:15 +0000
changeset 488207 ad04c84153da637000c7a472ccbc248116b5c330
parent 488206 f1d84baef0c346d83245fc92884d4132c921c99e
child 488208 caf7bd7998b21577f3ddce32f8d794d60e1ba203
push id36437
push userncsoregi@mozilla.com
push dateThu, 15 Aug 2019 19:33:18 +0000
treeherdermozilla-central@44aac6fc3352 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, jfkthame
bugs1571764
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1571764 - Subtract combobox display frame border-padding when inferring the height from line-height. r=mats,jfkthame This fixes it and seems to be an acceptable fix... Should I make it conditional on box-sizing: border-box for completeness? The display frame has border-box box-sizing, and not having it would be a bug, I'd think... Differential Revision: https://phabricator.services.mozilla.com/D41939
layout/forms/nsComboboxControlFrame.cpp
layout/reftests/forms/select/themed-select-padding-no-clip-ref.html
testing/web-platform/meta/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-2.html.ini
testing/web-platform/meta/html/rendering/replaced-elements/the-select-element/select-1-block-size-001.html.ini
testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-2.html
testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-ref-2.html
testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-ref.html
testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001.html
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1232,29 +1232,37 @@ NS_IMPL_FRAMEARENA_HELPERS(nsComboboxDis
 
 void nsComboboxDisplayFrame::Reflow(nsPresContext* aPresContext,
                                     ReflowOutput& aDesiredSize,
                                     const ReflowInput& aReflowInput,
                                     nsReflowStatus& aStatus) {
   MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!");
 
   ReflowInput state(aReflowInput);
+  WritingMode wm = aReflowInput.GetWritingMode();
+  LogicalMargin bp = state.ComputedLogicalBorderPadding();
   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.)
-    auto lh = ReflowInput::CalcLineHeight(mComboBox->GetContent(),
-                                          mComboBox->Style(), aPresContext,
-                                          NS_UNCONSTRAINEDSIZE, inflation);
+    nscoord lh = ReflowInput::CalcLineHeight(mComboBox->GetContent(),
+                                             mComboBox->Style(), aPresContext,
+                                             NS_UNCONSTRAINEDSIZE, inflation);
+    if (!mComboBox->StyleText()->mLineHeight.IsNormal()) {
+      // If the author specified a different line-height than normal, or a
+      // different appearance, subtract the border-padding from the
+      // comboboxdisplay frame, so as to respect that line-height rather than
+      // that line-height + 2px (from the UA sheet).
+      lh = std::max(0, lh - bp.BStartEnd(wm));
+    }
     state.SetComputedBSize(lh);
   }
-  WritingMode wm = aReflowInput.GetWritingMode();
-  nscoord inlineBp = state.ComputedLogicalBorderPadding().IStartEnd(wm);
+  nscoord inlineBp = bp.IStartEnd(wm);
   nscoord computedISize = mComboBox->mDisplayISize - inlineBp;
 
   // Other UAs ignore padding in some (but not all) platforms for (themed only)
   // comboboxes. Instead of doing that, we prevent that padding if present from
   // clipping the display text, by enforcing the display text minimum size in
   // that situation.
   const bool shouldHonorMinISize =
       mComboBox->StyleDisplay()->mAppearance == StyleAppearance::Menulist;
--- a/layout/reftests/forms/select/themed-select-padding-no-clip-ref.html
+++ b/layout/reftests/forms/select/themed-select-padding-no-clip-ref.html
@@ -14,17 +14,17 @@
 <!--- Ensure the text isn't chopped -->
 <select><option>XXXXXXXXXXXX</option></select>
 <select><option>X XXXXXXXXXX</option></select>
 
 <!--- Test that the display text doesn't overflow the select. -->
 <select style="overflow: hidden"><option>XXXXXXXXXXXXXXXXX</option></select>
 
 <!-- Test that this only affects themed comboboxes -->
-<div class="unthemed" style="padding-right: 15ch"><div style="padding: 1px 4px; overflow: hidden;">XXXXXXXXXX</div></div>
+<div class="unthemed" style="padding-right: 15ch"><div style="padding: 0 4px; overflow: hidden;">XXXXXXXXXX</div></div>
 
 <select id="src" style="padding-left: 10ch; padding-right: 10ch"><option>XXXXXXXXXX</option></select>
 <select id="ref" style="padding-left: 10ch;"><option>XXXXXXXXXX</option></select>
 <script>
   // This one is a bit special, because we want `ref` to have the same size as
   // `src`, which for some reason, with or without the patch for this bug,
   // grows outside the specified width. Given this width depends on scrollbar,
   // width, which is platform dependent, we just calculate it from `src`, and
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-2.html.ini
@@ -0,0 +1,2 @@
+[select-1-block-size-001-2.html]
+  fuzzy: maxDifference=48;totalPixels=0-4
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/html/rendering/replaced-elements/the-select-element/select-1-block-size-001.html.ini
@@ -0,0 +1,2 @@
+[select-1-block-size-001.html]
+  fuzzy: maxDifference=48;totalPixels=0-4
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-2.html
@@ -0,0 +1,28 @@
+<!doctype html>
+<title>Select block size when line-height is specified</title>
+<!--
+  FIXME: This is just a copy of select-1-block-size-001.html, but if I move the
+  <link rel="match"> in this file to select-1-block-size-001-ref.html (which is
+  the "right" way to do this), fuzzy annotations do not work.
+-->
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1571764">
+<link rel="match" href="select-1-block-size-001-ref-2.html">
+<style>
+select {
+  -webkit-appearance: none;
+  appearance: none;
+
+  background: black;
+  color: black;
+
+  line-height: 100px;
+  width: 100px;
+
+  border: 0;
+  padding: 0;
+}
+</style>
+<select></select>
+<select><option>A</option></select>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-ref-2.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1571764">
+<style>
+div {
+  -webkit-appearance: none;
+  appearance: none;
+
+  background: black;
+  color: black;
+
+  line-height: 100px;
+  width: 100px;
+
+  border: 0;
+  padding: 0;
+
+  display: inline-block;
+}
+</style>
+<div>A</div>
+<div>A</div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001-ref.html
@@ -0,0 +1,22 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1571764">
+<style>
+button {
+  -webkit-appearance: none;
+  appearance: none;
+
+  background: black;
+  color: black;
+
+  line-height: 100px;
+  width: 100px;
+
+  border: 0;
+  padding: 0;
+}
+</style>
+<button>A</button>
+<button>A</button>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/the-select-element/select-1-block-size-001.html
@@ -0,0 +1,23 @@
+<!doctype html>
+<title>Select block size when line-height is specified</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1571764">
+<link rel="match" href="select-1-block-size-001-ref.html">
+<style>
+select {
+  -webkit-appearance: none;
+  appearance: none;
+
+  background: black;
+  color: black;
+
+  line-height: 100px;
+  width: 100px;
+
+  border: 0;
+  padding: 0;
+}
+</style>
+<select></select>
+<select><option>A</option></select>