Bug 1571764 - Subtract combobox display frame border-padding when inferring the height from line-height. r=mats,jfkthame
☠☠ backed out by 725ed6685414 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 14 Aug 2019 16:06:46 +0000
changeset 487960 bd44d2dd0c02c1e2ab10e73695ee3ef1da971aa6
parent 487959 71b7ef6e0ed8b224e2715f894887587b1a80b34e
child 487961 b607778b1fdad6ad8e024bdc2e36796fe61b111a
push id36434
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:44:30 +0000
treeherdermozilla-central@144fbfb409b7 [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/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/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,23 @@
+<!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">
+<link rel="match" href="select-1-block-size-001-ref-2.html">
+<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>