Bug 1561794 - Do not crop display text of themed comboboxes due to padding. r=dbaron a=RyanVM
This is a potential fix that I thought it was worth doing rather than
implementing Blink's platform-dependent silliness. This ensures that the display
frame always has enough space to display itself.
Note that it may still get clipped, if there's no room for both the display
frame and the button.
Differential Revision:
https://phabricator.services.mozilla.com/D37922
--- a/dom/base/test/test_bug320799.html
+++ b/dom/base/test/test_bug320799.html
@@ -47,23 +47,28 @@ 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,16 +223,17 @@ 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()}
@@ -823,16 +824,19 @@ void nsComboboxControlFrame::Reflow(nsPr
PresContext(), aReflowInput.mRenderingContext, wm);
if (buttonISize > aReflowInput.ComputedISize()) {
buttonISize = 0;
}
}
mDisplayISize = aReflowInput.ComputedISize() - buttonISize;
+ mMaxDisplayISize =
+ mDisplayISize + aReflowInput.ComputedLogicalPadding().IEnd(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) +
@@ -1240,22 +1244,34 @@ 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 computedISize = mComboBox->mDisplayISize -
- state.ComputedLogicalBorderPadding().IStartEnd(wm);
- if (computedISize < 0) {
- computedISize = 0;
+ 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);
}
- state.SetComputedISize(computedISize);
+
+ state.SetComputedISize(std::max(0, 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,16 +284,21 @@ 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
@@ -624,17 +624,18 @@ 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.
+ // internally, except for nsComboboxDisplayFrame, which still wants to honor
+ // min-inline-size even though it wants to trump inline-size.
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,8 +9,13 @@ 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
+
+# Android and Windows actually use the anonymous select > button (rather than
+# drawing the arrow as a background like Linux and Mac), so most of this test
+# doesn't apply since when over-constrained it gets zero-sized.
+skip-if(Android||winWidget) == themed-select-padding-no-clip.html themed-select-padding-no-clip-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/select/themed-select-padding-no-clip-ref.html
@@ -0,0 +1,37 @@
+<!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>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/select/themed-select-padding-no-clip.html
@@ -0,0 +1,26 @@
+<!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,18 +329,20 @@ 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% ! important;
- box-sizing: border-box ! important;
+ block-size: 100%;
+ /* Try to always display our own text */
+ min-inline-size: max-content;
+ box-sizing: border-box;
line-height: -moz-block-height;
}
option {
display: block;
float: none !important;
position: static !important;
min-block-size: 1em;