Backed out changeset 05c5e4d30587 (bug 1561794) for multiple failures e.g. test_bug320799.html. CLOSED TREE
authorCsoregi Natalia <ncsoregi@mozilla.com>
Wed, 17 Jul 2019 04:24:52 +0300
changeset 483048 db6e0e3c4d49afb2323c83f80ab1504e4cdf7491
parent 483047 8231ec90138863906788b5e73f8781012be30531
child 483049 920ebfd35011bedee14ea1c6138c7afcb19d3f91
push id90159
push userncsoregi@mozilla.com
push dateWed, 17 Jul 2019 01:32:36 +0000
treeherderautoland@db6e0e3c4d49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1561794, 320799
milestone70.0a1
backs out05c5e4d30587aeab3f566e07edda1a3e6d22c41e
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
Backed out changeset 05c5e4d30587 (bug 1561794) for multiple failures e.g. test_bug320799.html. CLOSED TREE
dom/base/test/test_bug320799.html
layout/forms/nsComboboxControlFrame.cpp
layout/forms/nsComboboxControlFrame.h
layout/generic/ReflowInput.h
layout/reftests/forms/select/reftest.list
layout/reftests/forms/select/themed-select-padding-no-clip-ref.html
layout/reftests/forms/select/themed-select-padding-no-clip.html
layout/style/res/forms.css
--- a/dom/base/test/test_bug320799.html
+++ b/dom/base/test/test_bug320799.html
@@ -47,28 +47,23 @@ https://bugzilla.mozilla.org/show_bug.cg
     <option>x</option>
     <option>x</option>
     <option>x</option>
     <option>x</option>
   </select>
   <select id="s3">
     <option>x</option>
   </select>
-  <select id="s4" style="width: 100px; box-sizing: border-box; border: 0; margin: 10px">
-    <option>This is a test, it really is a test I tell you</option>
-  </select>
 </p>
 <div id="content" style="display: none">
   
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for Bug 320799 **/
 is($("s").scrollWidth, 100, "Scroll width should not include dropdown contents");
 is($("s2").clientWidth, $("s3").clientWidth,
    "Client width should not depend on the dropdown's vertical scrollbar");
-
-is($("s4").scrollWidth, 100, "Scroll width should not include dropdown contents");
 </script>
 </pre>
 </body>
 </html>
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -223,17 +223,16 @@ static int32_t gReflowInx = -1;
 nsComboboxControlFrame::nsComboboxControlFrame(ComputedStyle* aStyle,
                                                nsPresContext* aPresContext)
     : nsBlockFrame(aStyle, aPresContext, kClassID),
       mDisplayFrame(nullptr),
       mButtonFrame(nullptr),
       mDropdownFrame(nullptr),
       mListControlFrame(nullptr),
       mDisplayISize(0),
-      mMaxDisplayISize(0),
       mRecentSelectedIndex(NS_SKIP_NOTIFY_INDEX),
       mDisplayedIndex(-1),
       mLastDropDownBeforeScreenBCoord(nscoord_MIN),
       mLastDropDownAfterScreenBCoord(nscoord_MIN),
       mDroppedDown(false),
       mInRedisplayText(false),
       mDelayedShowDropDown(false),
       mIsOpenInParentProcess(false){REFLOW_COUNTER_INIT()}
@@ -824,22 +823,16 @@ void nsComboboxControlFrame::Reflow(nsPr
         PresContext(), aReflowInput.mRenderingContext, wm);
     if (buttonISize > aReflowInput.ComputedISize()) {
       buttonISize = 0;
     }
   }
 
   mDisplayISize = aReflowInput.ComputedISize() - buttonISize;
 
-  // Note that not all of this size may actually be usable, since it's from the
-  // inline-start side of where the display frame starts, but that this isn't
-  // actually a problem because it gets clipped anyway. This is exclusively to
-  // avoid exposing the size of the contents inadvertently via DOM APIs.
-  mMaxDisplayISize = aReflowInput.ComputedSizeWithBorderPadding().ISize(wm);
-
   nsBlockFrame::Reflow(aPresContext, aDesiredSize, aReflowInput, aStatus);
 
   // The button should occupy the same space as a scrollbar
   nsSize containerSize = aDesiredSize.PhysicalSize();
   LogicalRect buttonRect = mButtonFrame->GetLogicalRect(containerSize);
 
   buttonRect.IStart(wm) =
       aReflowInput.ComputedLogicalBorderPadding().IStartEnd(wm) +
@@ -1247,34 +1240,22 @@ void nsComboboxDisplayFrame::Reflow(nsPr
     // (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 inlineBp = state.ComputedLogicalBorderPadding().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;
-  if (shouldHonorMinISize) {
-    computedISize = std::max(state.ComputedMinISize(), computedISize);
-    // Don't let this size go over mMaxDisplayISize, since that'd be
-    // observable via clientWidth / scrollWidth.
-    computedISize =
-        std::min(computedISize, mComboBox->mMaxDisplayISize - inlineBp);
+  nscoord computedISize = mComboBox->mDisplayISize -
+                          state.ComputedLogicalBorderPadding().IStartEnd(wm);
+  if (computedISize < 0) {
+    computedISize = 0;
   }
-
-  state.SetComputedISize(std::max(0, computedISize));
+  state.SetComputedISize(computedISize);
   nsBlockFrame::Reflow(aPresContext, aDesiredSize, state, aStatus);
   aStatus.Reset();  // this type of frame can't be split
 }
 
 void nsComboboxDisplayFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                               const nsDisplayListSet& aLists) {
   nsDisplayListCollection set(aBuilder);
   nsBlockFrame::BuildDisplayList(aBuilder, set);
--- a/layout/forms/nsComboboxControlFrame.h
+++ b/layout/forms/nsComboboxControlFrame.h
@@ -284,21 +284,16 @@ class nsComboboxControlFrame final : pub
   nsContainerFrame* mDisplayFrame;     // frame to display selection
   nsIFrame* mButtonFrame;              // button frame
   nsIFrame* mDropdownFrame;            // dropdown list frame
   nsListControlFrame* mListControlFrame;  // ListControl for the dropdown frame
 
   // The inline size of our display area.  Used by that frame's reflow
   // to size to the full inline size except the drop-marker.
   nscoord mDisplayISize;
-  // The maximum inline size of our display area, which is the
-  // nsComoboxControlFrame's border-box.
-  //
-  // Going over this would be observable via DOM APIs like client / scrollWidth.
-  nscoord mMaxDisplayISize;
 
   nsRevocableEventPtr<RedisplayTextEvent> mRedisplayTextEvent;
 
   int32_t mRecentSelectedIndex;
   int32_t mDisplayedIndex;
   nsString mDisplayedOptionTextOrPreview;
 
   // make someone to listen to the button. If its programmatically pressed by
--- a/layout/generic/ReflowInput.h
+++ b/layout/generic/ReflowInput.h
@@ -637,18 +637,17 @@ struct ReflowInput : public SizeComputat
   nscoord mComputedHeight;
 
   // Computed values for 'left/top/right/bottom' offsets. Only applies to
   // 'positioned' elements. These are PHYSICAL coordinates (for now).
   nsMargin mComputedOffsets;
 
   // Computed values for 'min-width/max-width' and 'min-height/max-height'
   // XXXldb The width ones here should go; they should be needed only
-  // internally, except for nsComboboxDisplayFrame, which still wants to honor
-  // min-inline-size even though it wants to trump inline-size.
+  // internally.
   MOZ_INIT_OUTSIDE_CTOR
   nscoord mComputedMinWidth, mComputedMaxWidth;
   MOZ_INIT_OUTSIDE_CTOR
   nscoord mComputedMinHeight, mComputedMaxHeight;
 
  public:
   // Our saved containing block dimensions.
   LogicalSize mContainingBlockSize = LogicalSize(mWritingMode);
--- a/layout/reftests/forms/select/reftest.list
+++ b/layout/reftests/forms/select/reftest.list
@@ -9,9 +9,8 @@ fuzzy(0-1,0-4) == padding-button-placeme
 fuzzy-if(skiaContent,0-4,0-1) needs-focus == focusring-1.html focusring-1-ref.html
 needs-focus == focusring-2.html focusring-2-ref.html
 needs-focus == focusring-3.html focusring-3-ref.html
 == dynamic-text-indent-1.html dynamic-text-indent-1-ref.html
 == dynamic-text-overflow-1.html dynamic-text-overflow-1-ref.html
 == listbox-zero-row-initial.html listbox-zero-row-initial-ref.html
 == 1246836.html 1246836-ref.html
 skip-if(Android) == select-option-display-none-inline-size.html select-option-display-none-inline-size-ref.html
-skip-if(Android) == themed-select-padding-no-clip.html themed-select-padding-no-clip-ref.html
deleted file mode 100644
--- a/layout/reftests/forms/select/themed-select-padding-no-clip-ref.html
+++ /dev/null
@@ -1,37 +0,0 @@
-<!doctype html>
-<title>Bug 1561794 - padding-inline-end does not clip display text for themed comboboxes.</title>
-<style>
-  select, .unthemed {
-    font: 16px / 1 monospace;
-    width: 20ch;
-    display: block;
-  }
-  .unthemed {
-    box-sizing: border-box;
-    border: 1px solid grey;
-  }
-</style>
-<!--- 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>
-
-<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
-  // then hide it.
-  //
-  // We just really want to test that the padding right doesn't clip the text
-  // anyhow.
-  ref.style.width = getComputedStyle(src).width;
-  src.style.display = "none";
-</script>
deleted file mode 100644
--- a/layout/reftests/forms/select/themed-select-padding-no-clip.html
+++ /dev/null
@@ -1,26 +0,0 @@
-<!doctype html>
-<title>Bug 1561794 - padding-inline-end does not clip display text for themed comboboxes.</title>
-<style>
-  select {
-    font: 16px / 1 monospace;
-    width: 20ch;
-    display: block;
-  }
-  .unthemed {
-    -moz-appearance: none;
-    -webkit-appearance: none;
-    border: 1px solid grey;
-    background: transparent;
-  }
-</style>
-<!--- Ensure the text isn't chopped -->
-<select style="padding-right: 15ch"><option>XXXXXXXXXXXX</option></select>
-<select style="padding-right: 15ch"><option>X XXXXXXXXXX</option></select>
-
-<!--- Test that the display text doesn't overflow the select. It'd be nice to test it doesn't overlap the button (which is true), but that seems hard to test via a reftest -->
-<select style="padding-right: 15ch"><option>XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX</option></select>
-
-<!-- Test that this only affects themed comboboxes; 4px is the padding-inline-start of the comboboxcontrol-frame anon box, so as to clip exactly at a character boundary. -->
-<select class="unthemed" style="color: initial; padding-right: 15ch"><option>XXXXXXXXXX</option></select>
-
-<select style="padding-left: 10ch; padding-right: 10ch"><option>XXXXXXXXXX</option></select>
--- a/layout/style/res/forms.css
+++ b/layout/style/res/forms.css
@@ -329,20 +329,18 @@ select > button[orientation="right"]:act
   padding-block-end: 1px;
   padding-inline-start: 4px;
   padding-inline-end: 4px;
   color: unset;
   white-space: nowrap;
   text-align: unset;
   user-select: none;
   /* Make sure to size correctly if the combobox has a non-auto block-size. */
-  block-size: 100%;
-  /* Try to always display our own text */
-  min-inline-size: max-content;
-  box-sizing: border-box;
+  block-size: 100% ! important;
+  box-sizing: border-box ! important;
   line-height: -moz-block-height;
 }
 
 option {
   display: block;
   float: none !important;
   position: static !important;
   min-block-size: 1em;