Bug 1174003 part 5: [css-flexbox] Remove is-{main,cross}-axis-horizontal checks from ReflowFlexItem. r?mats
This patch mostly[1] doesn't affect behavior. It just changes some ReflowInput
width/height-setting logic to use ISize/BSize setters instead of width/height
setters.
To help with this, I'm also adding some more getters to answer the question "is
this flex item's {inline,block} axis the same as the flex container's
{main,cross} axis", for convenience/readability at callsites. (All of these
getters are simply giving a different view of the same underlying single bit of
information.)
[1] One way this patch *kind of* affects behavior: it generalizes the
conditions under which we set the NS_FRAME_CONTAINS_RELATIVE_BSIZE flag on a
flex item. But that doesn't actually have any user-visible effects right now,
because that flag's only purpose is to determine whether we should reflow, and
currently we always reflow all flex items. If/when that changes, though
(i.e. when we start reflowing flex items more selectively), this patch is
adding a reftest that may test this generalized behavior. (The reftest actually
trivially passes right now, due to unrelated
bug 1441348.)
MozReview-Commit-ID: 4NEKLBAjowh
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -565,24 +565,27 @@ public:
// min-[width|height] property.
bool NeedsMinSizeAutoResolution() const
{ return mNeedsMinSizeAutoResolution; }
// Indicates whether this item is a "strut" left behind by an element with
// visibility:collapse.
bool IsStrut() const { return mIsStrut; }
- // Returns true if this item's inline axis is parallel (or antiparallel)
- // to the container's main axis. Otherwise (i.e. if this item's inline axis
- // is orthogonal to the container's main axis), this function returns false.
- bool IsInlineAxisMainAxis() const { return mIsInlineAxisMainAxis; }
-
- // Same as above, but for cross axis. Equivalent to !IsInlineAxisMainAxis().
- // This just exists for convenience/readability at callsites.
+ // IsInlineAxisMainAxis() returns true if this item's inline axis is parallel
+ // (or antiparallel) to the container's main axis. Otherwise (i.e. if this
+ // item's inline axis is orthogonal to the container's main axis), this
+ // function returns false. The next 3 methods are all other ways of asking
+ // the same question, and only exist for readability at callsites (depending
+ // on which axes those callsites are reasoning about).
+ bool IsInlineAxisMainAxis() const { return mIsInlineAxisMainAxis; }
bool IsInlineAxisCrossAxis() const { return !mIsInlineAxisMainAxis; }
+ bool IsBlockAxisMainAxis() const { return !mIsInlineAxisMainAxis; }
+ bool IsBlockAxisCrossAxis() const { return mIsInlineAxisMainAxis; }
+
WritingMode GetWritingMode() const { return mWM; }
uint8_t GetAlignSelf() const { return mAlignSelf; }
// Returns the flex factor (flex-grow or flex-shrink), depending on
// 'aIsUsingFlexGrow'.
//
// Asserts fatally if called on a frozen item (since frozen items are not
@@ -4858,71 +4861,79 @@ nsFlexContainerFrame::ReflowFlexItem(nsP
{
WritingMode outerWM = aReflowInput.GetWritingMode();
WritingMode wm = aItem.Frame()->GetWritingMode();
LogicalSize availSize = aReflowInput.ComputedSize(wm);
availSize.BSize(wm) = NS_UNCONSTRAINEDSIZE;
ReflowInput childReflowInput(aPresContext, aReflowInput,
aItem.Frame(), availSize);
- // Keep track of whether we've overriden the child's computed height
- // and/or width, so we can set its resize flags accordingly.
- bool didOverrideComputedWidth = false;
- bool didOverrideComputedHeight = false;
+ // Keep track of whether we've overriden the child's computed ISize
+ // and/or BSize, so we can set its resize flags accordingly.
+ bool didOverrideComputedISize = false;
+ bool didOverrideComputedBSize = false;
// Override computed main-size
- if (aAxisTracker.IsMainAxisHorizontal()) {
- childReflowInput.SetComputedWidth(aItem.GetMainSize());
- didOverrideComputedWidth = true;
+ if (aItem.IsInlineAxisMainAxis()) {
+ childReflowInput.SetComputedISize(aItem.GetMainSize());
+ didOverrideComputedISize = true;
} else {
- childReflowInput.SetComputedHeight(aItem.GetMainSize());
- didOverrideComputedHeight = true;
+ childReflowInput.SetComputedBSize(aItem.GetMainSize());
+ didOverrideComputedBSize = true;
}
// Override reflow state's computed cross-size if either:
// - the item was stretched (in which case we're imposing a cross size)
// ...or...
// - the item it has an aspect ratio (in which case the cross-size that's
// currently in the reflow state is based on arithmetic involving a stale
// main-size value that we just stomped on above). (Note that we could handle
// this case using an AutoFlexItemMainSizeOverride, as we do elsewhere; but
// given that we *already know* the correct cross size to use here, it's
// cheaper to just directly set it instead of setting a frame property.)
if (aItem.IsStretched() ||
aItem.HasIntrinsicRatio()) {
- if (aAxisTracker.IsCrossAxisHorizontal()) {
- childReflowInput.SetComputedWidth(aItem.GetCrossSize());
- didOverrideComputedWidth = true;
+ if (aItem.IsInlineAxisCrossAxis()) {
+ childReflowInput.SetComputedISize(aItem.GetCrossSize());
+ didOverrideComputedISize = true;
} else {
- childReflowInput.SetComputedHeight(aItem.GetCrossSize());
- didOverrideComputedHeight = true;
+ childReflowInput.SetComputedBSize(aItem.GetCrossSize());
+ didOverrideComputedBSize = true;
}
}
- if (aItem.IsStretched() && !aAxisTracker.IsCrossAxisHorizontal()) {
- // If this item's height is stretched, it's a relative height.
+ if (aItem.IsStretched() && aItem.IsBlockAxisCrossAxis()) {
+ // This item is stretched (in the cross axis), and that axis is its block
+ // axis. That stretching effectively gives it a relative BSize.
+ // XXXdholbert This flag only makes a difference if we use the flex items'
+ // frame-state when deciding whether to reflow them -- and we don't, as of
+ // the changes in bug 851607. So this has no effect right now, but it might
+ // make a difference if we optimize to use dirty bits in the
+ // future. (Reftests flexbox-resizeviewport-1.xhtml and -2.xhtml are
+ // intended to catch any regressions here, if we end up relying on this bit
+ // & neglecting to set it.)
aItem.Frame()->AddStateBits(NS_FRAME_CONTAINS_RELATIVE_BSIZE);
}
// XXXdholbert Might need to actually set the correct margins in the
// reflow state at some point, so that they can be saved on the frame for
// UsedMarginProperty(). Maybe doesn't matter though...?
// If we're overriding the computed width or height, *and* we had an
// earlier "measuring" reflow, then this upcoming reflow needs to be
// treated as a resize.
if (aItem.HadMeasuringReflow()) {
- if (didOverrideComputedWidth) {
- // (This is somewhat redundant, since the reflow state already
- // sets mHResize whenever our computed width has changed since the
- // previous reflow. Still, it's nice for symmetry, and it may become
- // necessary once we support orthogonal flows.)
- childReflowInput.SetHResize(true);
+ if (didOverrideComputedISize) {
+ // (This is somewhat redundant, since ReflowInput::InitResizeFlags()
+ // already calls SetIResize() whenever our computed ISize has changed
+ // since the previous reflow. Still, it's nice for symmetry, and it might
+ // be necessary for some edge cases.)
+ childReflowInput.SetIResize(true);
}
- if (didOverrideComputedHeight) {
- childReflowInput.SetVResize(true);
+ if (didOverrideComputedBSize) {
+ childReflowInput.SetBResize(true);
}
}
// NOTE: Be very careful about doing anything else with childReflowInput
// after this point, because some of its methods (e.g. SetComputedWidth)
// internally call InitResizeFlags and stomp on mVResize & mHResize.
ReflowOutput childDesiredSize(childReflowInput);
nsReflowStatus childReflowStatus;
copy from layout/reftests/flexbox/flexbox-resizeviewport-1-helper.html
copy to layout/reftests/flexbox/flexbox-resizeviewport-2-helper.html
--- a/layout/reftests/flexbox/flexbox-resizeviewport-1-helper.html
+++ b/layout/reftests/flexbox/flexbox-resizeviewport-2-helper.html
@@ -1,21 +1,22 @@
<!--
Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/
-->
-<!-- Helper-file for reftest flexbox-resizeviewport-1.xhtml
+<!-- Helper-file for reftest flexbox-resizeviewport-2.xhtml
I'm intentionally using quirks-mode (no doctype), so that
- a 100% height will work. -->
+ a 100% width (block-size) will work. -->
<html>
<head>
<style>
+ html { writing-mode: vertical-rl; }
div.flexbox {
display: flex;
- height: 100%;
+ width: 100%;
border: 2px dashed black;
}
div.a {
flex: 1;
background: pink;
}
div.b {
flex: 1;
copy from layout/reftests/flexbox/flexbox-resizeviewport-1-ref.xhtml
copy to layout/reftests/flexbox/flexbox-resizeviewport-2-ref.xhtml
--- a/layout/reftests/flexbox/flexbox-resizeviewport-1-ref.xhtml
+++ b/layout/reftests/flexbox/flexbox-resizeviewport-2-ref.xhtml
@@ -8,15 +8,15 @@
<style>
iframe {
width: 75px;
height: 75px;
}
</style>
</head>
<body>
- <iframe src="flexbox-resizeviewport-1-helper.html" style="width: 50px"/>
- <iframe src="flexbox-resizeviewport-1-helper.html" style="width: 125px"/>
+ <iframe src="flexbox-resizeviewport-2-helper.html" style="width: 50px"/>
+ <iframe src="flexbox-resizeviewport-2-helper.html" style="width: 125px"/>
<br/>
- <iframe src="flexbox-resizeviewport-1-helper.html" style="height: 50px"/>
- <iframe src="flexbox-resizeviewport-1-helper.html" style="height: 125px"/>
+ <iframe src="flexbox-resizeviewport-2-helper.html" style="height: 50px"/>
+ <iframe src="flexbox-resizeviewport-2-helper.html" style="height: 125px"/>
</body>
</html>
copy from layout/reftests/flexbox/flexbox-resizeviewport-1.xhtml
copy to layout/reftests/flexbox/flexbox-resizeviewport-2.xhtml
--- a/layout/reftests/flexbox/flexbox-resizeviewport-1.xhtml
+++ b/layout/reftests/flexbox/flexbox-resizeviewport-2.xhtml
@@ -1,15 +1,19 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/
-->
<!-- Testcase to be sure a flex container gets reflowed properly when its
iframe changes size. -->
+<!-- XXXdholbert This testcase can't actually test the scenario it's intended
+ to test right now (quirks-mode percent-BSize resolution in a vertical
+ writing mode), due to Bug 1441348.
+ -->
<html xmlns="http://www.w3.org/1999/xhtml"
class="reftest-wait">
<head>
<style>
iframe {
width: 75px;
height: 75px;
}
@@ -26,15 +30,15 @@
setElementPropertyTo("c", "height", "50px");
setElementPropertyTo("d", "height", "125px");
document.documentElement.removeAttribute("class");
}
window.addEventListener("MozReftestInvalidate", tweak, false);
</script>
</head>
<body>
- <iframe id="a" src="flexbox-resizeviewport-1-helper.html"/>
- <iframe id="b" src="flexbox-resizeviewport-1-helper.html"/>
+ <iframe id="a" src="flexbox-resizeviewport-2-helper.html"/>
+ <iframe id="b" src="flexbox-resizeviewport-2-helper.html"/>
<br/>
- <iframe id="c" src="flexbox-resizeviewport-1-helper.html"/>
- <iframe id="d" src="flexbox-resizeviewport-1-helper.html"/>
+ <iframe id="c" src="flexbox-resizeviewport-2-helper.html"/>
+ <iframe id="d" src="flexbox-resizeviewport-2-helper.html"/>
</body>
</html>
--- a/layout/reftests/flexbox/reftest.list
+++ b/layout/reftests/flexbox/reftest.list
@@ -106,16 +106,17 @@ fails == flexbox-inlinecontent-horiz-1b.
== flexbox-intrinsic-sizing-horiz-2a.xhtml flexbox-intrinsic-sizing-horiz-2-ref.xhtml
== flexbox-intrinsic-sizing-horiz-2b.xhtml flexbox-intrinsic-sizing-horiz-2-ref.xhtml
# Tests for invalidation after dynamic modifications
== flexbox-invalidation-1.html flexbox-invalidation-1-ref.html
# Tests for flexbox in an iframe that gets resized.
fuzzy-if(skiaContent,1,5) == flexbox-resizeviewport-1.xhtml flexbox-resizeviewport-1-ref.xhtml
+fuzzy-if(skiaContent,1,5) == flexbox-resizeviewport-2.xhtml flexbox-resizeviewport-2-ref.xhtml
# Tests for flexbox styling on things that don't support it
== flexbox-styling-on-svg-1.svg flexbox-styling-on-svg-1-ref.svg
# Tests with widgets as flex items
fuzzy-if(gtkWidget,1,66) == flexbox-widget-flex-items-1.html flexbox-widget-flex-items-1-ref.html
fuzzy-if(gtkWidget,1,74) == flexbox-widget-flex-items-2.html flexbox-widget-flex-items-2-ref.html
skip-if(gtkWidget) == flexbox-widget-flex-items-3.html flexbox-widget-flex-items-3-ref.html # bug 1260965