Bug 1293242 - Part 2 - Propagate resized child size up to parent scrollable frame. r=dbaron
Currently XUL flexbox frames expand the size of themselves when a child is resized during layout (typically due to text that now wraps). This happens near the end of nsSprocketLayout::XULLayout. When this happens and the frame is the direct child of a scrollable frame, propagate this expanded size change up to the parent scrollable frame.
This fixes issues with the transition in the downloads panel that contains a number of wrapping blocks inside a scrollable container that doesn't expand to the full height of the contents. The test_bug393970.xul asserts more on Linux, but the layout of the grid in that test now looks correct, whereas before it was cut off.
MozReview-Commit-ID: 8B7XNJYxopS
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -1161,16 +1161,18 @@ NS_IMPL_FRAMEARENA_HELPERS(nsXULScrollFr
nsXULScrollFrame::nsXULScrollFrame(nsStyleContext* aContext,
bool aIsRoot,
bool aClipAllDescendants)
: nsBoxFrame(aContext, LayoutFrameType::Scroll, aIsRoot)
, mHelper(ALLOW_THIS_IN_INITIALIZER_LIST(this), aIsRoot)
{
SetXULLayoutManager(nullptr);
mHelper.mClipAllDescendants = aClipAllDescendants;
+ mChildExpandedHorizontally = false;
+ mChildExpandedVertically = false;
}
void
nsXULScrollFrame::ScrollbarActivityStarted() const
{
if (mHelper.mScrollbarActivity) {
mHelper.mScrollbarActivity->ActivityStarted();
}
@@ -4924,16 +4926,27 @@ nsXULScrollFrame::LayoutScrollArea(nsBox
// REVIEW: Have we accounted for both?
ClampAndSetBounds(aState, childRect, aScrollPosition, true);
}
aState.SetLayoutFlags(oldflags);
}
+void
+nsXULScrollFrame::ScrolledFrameResized(nsBoxLayoutState& aState,
+ nsIFrame* aFrame, nsSize aSize,
+ bool aHorizontal, bool aVertical)
+{
+ MOZ_ASSERT(aFrame == mHelper.mScrolledFrame);
+
+ mChildExpandedHorizontally = aHorizontal;
+ mChildExpandedVertically = aVertical;
+}
+
void ScrollFrameHelper::PostOverflowEvent()
{
if (mAsyncScrollPortEvent.IsPending()) {
return;
}
// Keep this in sync with FireScrollPortEvent().
nsSize scrollportSize = mScrollPort.Size();
@@ -5095,19 +5108,77 @@ nsXULScrollFrame::XULLayout(nsBoxLayoutS
mHelper.mHasVerticalScrollbar = true;
if (mHelper.mHasHorizontalScrollbar)
AddHorizontalScrollbar(aState, scrollbarBottom);
if (mHelper.mHasVerticalScrollbar)
AddVerticalScrollbar(aState, scrollbarRight);
+ mChildExpandedHorizontally = false;
+ mChildExpandedVertically = false;
+
// layout our the scroll area
LayoutScrollArea(aState, oldScrollPosition);
+ // If mChildExpandedHorizontally or mChildExpandedVertically is set, then
+ // the scrolled frame resized during layout, likely due to containing some
+ // text that needed to be wrapped. In this case get the scrolled frame's
+ // new size and expand the scroll frame to match.
+ nsRect finalRect;
+ nsRect finalPortRect;
+ if (mChildExpandedHorizontally || mChildExpandedVertically) {
+ // If an explicit width or height was assigned then just use that size
+ // instead of expanding in that direction.
+ bool widthSet, heightSet;
+ nsSize prefSizeCSS(NS_INTRINSICSIZE, NS_INTRINSICSIZE);
+ nsIFrame::AddXULPrefSize(this, prefSizeCSS, widthSet, heightSet);
+
+ nsSize scrolledSize = mHelper.mScrolledFrame->GetRect().Size();
+ nsMargin borderPadding;
+ GetXULBorderAndPadding(borderPadding);
+
+ nsSize scrollbarExpandSize;
+
+ nsRect newRect = GetRect();
+ if (!widthSet && mChildExpandedHorizontally) {
+ // Use the scrolled width plus our border and padding plus any visible scrollbar.
+ nsSize scrollbarSize;
+ if (mHelper.mHasVerticalScrollbar && mHelper.mVScrollbarBox) {
+ scrollbarSize = mHelper.mVScrollbarBox->GetXULPrefSize(aState);
+ nsBox::AddMargin(mHelper.mVScrollbarBox, scrollbarSize);
+ scrollbarExpandSize.width = scrollbarSize.width;
+ }
+
+ newRect.width = scrolledSize.width + borderPadding.LeftRight() + scrollbarSize.width;
+ }
+ if (!heightSet && mChildExpandedVertically) {
+ // Use the scrolled height plus our border and padding plus any visible scrollbar.
+ nsSize scrollbarSize;
+ if (mHelper.mHasHorizontalScrollbar && mHelper.mHScrollbarBox) {
+ scrollbarSize = mHelper.mHScrollbarBox->GetXULPrefSize(aState);
+ nsBox::AddMargin(mHelper.mHScrollbarBox, scrollbarSize);
+ scrollbarExpandSize.height = scrollbarSize.height;
+ }
+
+ newRect.height = scrolledSize.height + borderPadding.TopBottom() + scrollbarSize.height;
+ }
+
+ SetXULBounds(aState, newRect);
+
+ // Now set the scrollport size to match the newly assigned size.
+ GetXULClientRect(clientRect);
+ if (mChildExpandedHorizontally) {
+ mHelper.mScrollPort.width = clientRect.width - scrollbarExpandSize.width;
+ }
+ if (mChildExpandedVertically) {
+ mHelper.mScrollPort.height = clientRect.height - scrollbarExpandSize.height;
+ }
+ }
+
// now look at the content area and see if we need scrollbars or not
bool needsLayout = false;
// if we have 'auto' scrollbars look at the vertical case
if (styles.mVertical != NS_STYLE_OVERFLOW_SCROLL) {
// These are only good until the call to LayoutScrollArea.
nsRect scrolledRect = mHelper.GetScrolledRect();
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -1045,16 +1045,22 @@ public:
// Update the style on our scrolled frame.
virtual void DoUpdateStyleOfOwnedAnonBoxes(mozilla::ServoStyleSet& aStyleSet,
nsStyleChangeList& aChangeList,
nsChangeHint aHintForThisFrame) override {
UpdateStyleOfChildAnonBox(mHelper.GetScrolledFrame(), aStyleSet,
aChangeList, aHintForThisFrame);
}
+ virtual void ScrolledFrameResized(nsBoxLayoutState& aState, nsIFrame* aFrame,
+ nsSize aSize, bool aHorizontal, bool aVertical) override {
+ // This should never be called for HTML scroll frames.
+ MOZ_ASSERT(false);
+ };
+
#ifdef DEBUG_FRAME_DUMP
virtual nsresult GetFrameName(nsAString& aResult) const override;
#endif
#ifdef ACCESSIBILITY
virtual mozilla::a11y::AccType AccessibleType() override;
#endif
@@ -1489,16 +1495,19 @@ public:
virtual void DoUpdateStyleOfOwnedAnonBoxes(mozilla::ServoStyleSet& aStyleSet,
nsStyleChangeList& aChangeList,
nsChangeHint aHintForThisFrame) override {
UpdateStyleOfChildAnonBox(mHelper.GetScrolledFrame(), aStyleSet,
aChangeList, aHintForThisFrame);
}
+ virtual void ScrolledFrameResized(nsBoxLayoutState& aState, nsIFrame* aFrame,
+ nsSize aSize, bool aHorizontal, bool aVertical) override;
+
#ifdef DEBUG_FRAME_DUMP
virtual nsresult GetFrameName(nsAString& aResult) const override;
#endif
protected:
nsXULScrollFrame(nsStyleContext* aContext, bool aIsRoot,
bool aClipAllDescendants);
@@ -1511,14 +1520,17 @@ protected:
* edge, then subtract the current width to find the physical position.
*/
if (!mHelper.IsPhysicalLTR()) {
aRect.x = mHelper.mScrollPort.XMost() - aScrollPosition.x - aRect.width;
}
mHelper.mScrolledFrame->SetXULBounds(aState, aRect, aRemoveOverflowAreas);
}
+ bool mChildExpandedHorizontally;
+ bool mChildExpandedVertically;
+
private:
friend class mozilla::ScrollFrameHelper;
ScrollFrameHelper mHelper;
};
#endif /* nsGfxScrollFrame_h___ */
--- a/layout/generic/nsIScrollableFrame.h
+++ b/layout/generic/nsIScrollableFrame.h
@@ -487,11 +487,19 @@ public:
virtual void AsyncScrollbarDragRejected() = 0;
/**
* Returns whether this scroll frame is the root scroll frame of the document
* that it is in. Note that some documents don't have root scroll frames at
* all (ie XUL documents) even though they may contain other scroll frames.
*/
virtual bool IsRootScrollFrameOfDocument() const = 0;
+
+ /**
+ * Called by a child XUL flexbox to indicate that it had to grow due to a
+ * child block growing. This should increase the size of the scrolled frame.
+ * This should be ignored by other frame types.
+ */
+ virtual void ScrolledFrameResized(nsBoxLayoutState& aState, nsIFrame* aFrame,
+ nsSize aSize, bool aHorizontal, bool aVertical) = 0;
};
#endif
new file mode 100644
--- /dev/null
+++ b/layout/reftests/xul/description-inside-overflowhidden-1-ref.xul
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<!-- Height not set in CSS, no scrollframe. -->
+<vbox style="max-width: 150px;">
+ <description style="width: 150px;"><span style="width: 149px;">Line 1</span> Line 2</description>
+</vbox>
+
+</window>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/xul/description-inside-overflowhidden-1.xul
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<!-- Height not set in CSS, the scrollframe should expand. -->
+<vbox style="max-width: 150px; overflow: hidden;">
+ <description style="width: 150px;"><span style="width: 149px;">Line 1</span> Line 2</description>
+</vbox>
+
+</window>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/xul/description-inside-overflowhidden-2-ref.xul
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<!-- Height set in CSS, the scrollframe should not expand. -->
+<vbox style="max-width: 150px; background: green; height: 7px; overflow: hidden;">
+ <!-- This does not wrap, so it does not trigger scrollframe expansion code. -->
+ <description style="width: 150px;" value="Line 1" />
+</vbox>
+
+</window>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/xul/description-inside-overflowhidden-2.xul
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<!-- Height set in CSS, the scrollframe should not expand. -->
+<vbox style="max-width: 150px; background: green; height: 7px; overflow: hidden;">
+ <!-- This wraps, so it triggers scrollframe expansion code. -->
+ <description style="width: 150px;"><span style="width: 149px;">Line 1</span> Line 2</description>
+</vbox>
+
+</window>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/xul/description-inside-overflowhidden-3-ref.xul
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<!-- Height constrained indirectly, the scrollframe should not expand. -->
+<vbox style="height: 7px;">
+ <vbox flex="1" style="max-width: 150px; background: green; overflow: hidden;">
+ <!-- This does not wrap, so it does not trigger scrollframe expansion code. -->
+ <description style="width: 150px;" value="Line 1" />
+ </vbox>
+</vbox>
+
+</window>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/xul/description-inside-overflowhidden-3.xul
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+<!-- Height constrained indirectly, the scrollframe should not expand. -->
+<vbox style="height: 7px;">
+ <vbox flex="1" style="max-width: 150px; background: green; overflow: hidden;">
+ <!-- This wraps, so it triggers scrollframe expansion code. -->
+ <description style="width: 150px;"><span style="width: 149px;">Line 1</span> Line 2</description>
+ </vbox>
+</vbox>
+
+</window>
--- a/layout/reftests/xul/reftest.list
+++ b/layout/reftests/xul/reftest.list
@@ -65,8 +65,13 @@ fuzzy-if(webrender,16,20) == object-posi
# Tests for rendering SVG images in a XUL <treecell>:
# XXXdholbert: These are marked as "random" right now, since they might not
# render the images they trying to test in time for the reftest snapshot, per
# bug 1218954.
skip == treecell-image-svg-1a.xul treecell-image-svg-1-ref.xul # bug 1218954
skip == treecell-image-svg-1b.xul treecell-image-svg-1-ref.xul # bug 1218954
== treechildren-padding-percent-1.xul treechildren-padding-percent-1-ref.xul
+
+# These tests verify the size of overflow: hidden elements with a wrapping block inside.
+== description-inside-overflowhidden-1.xul description-inside-overflowhidden-1-ref.xul
+== description-inside-overflowhidden-2.xul description-inside-overflowhidden-2-ref.xul
+== description-inside-overflowhidden-3.xul description-inside-overflowhidden-3-ref.xul
--- a/layout/xul/nsSprocketLayout.cpp
+++ b/layout/xul/nsSprocketLayout.cpp
@@ -604,16 +604,25 @@ nsSprocketLayout::XULLayout(nsIFrame* aB
nsRect bounds(aBox->GetRect());
if (tmpClientRect.width > originalSize.width)
bounds.width = tmpClientRect.width;
if (tmpClientRect.height > originalSize.height)
bounds.height = tmpClientRect.height;
aBox->SetXULBounds(aState, bounds);
+
+ // If inside a scrollframe, increase the size of the scrollframe
+ // to the same size as well.
+ nsIScrollableFrame* scrollableParent = do_QueryFrame(aBox->GetParent());
+ if (scrollableParent) {
+ scrollableParent->ScrolledFrameResized(aState, aBox, bounds.Size(),
+ tmpClientRect.width > originalSize.width,
+ tmpClientRect.height > originalSize.height);
+ }
}
}
// Because our size grew, we now have to readjust because of box packing. Repack
// in order to update our x and y to the correct values.
HandleBoxPack(aBox, frameState, x, y, originalClientRect, clientRect);
// Compare against our original x and y and only worry about adjusting the children if
--- a/layout/xul/test/test_bug393970.xul
+++ b/layout/xul/test/test_bug393970.xul
@@ -46,17 +46,17 @@ https://bugzilla.mozilla.org/show_bug.cg
</grid>
</hbox>
<!-- test code goes here -->
<script type="application/javascript"><![CDATA[
/** Test for Bug 393970 **/
if (navigator.platform.startsWith("Linux")) {
- SimpleTest.expectAssertions(0, 24);
+ SimpleTest.expectAssertions(0, 48);
}
var tests = [
'overflow-x: hidden; overflow-y: hidden;',
'overflow-x: scroll; overflow-y: hidden;',
'overflow-x: hidden; overflow-y: scroll;',
'overflow-x: scroll; overflow-y: scroll;',
];