Bug 1347759 - Fix conditions under which we reflow absolutely positioned element due to size change of its container for everything other than horizontal LTR. r=jfkthame

I found this problem because I was debugging the failure of
layout/reftests/w3c-css/received/css-writing-modes-3/clearance-calculations-vrl-008.xht
with my patch for bug 1308876.  It was failing because the red reference
box that was intended to be covered up was being mispositioned leftwards
by the width of the scrollbar, since we were not reflowing it when we
decided that the viewport did not need scrollbars.  This patch fixes
that failure.

This led me to this bug, where
nsAbsoluteContainingBlock::FrameDependsOnContainer was incorrectly
testing conditions for when the values of 'top', 'right', 'bottom', and
'left' require reflow due to changes in the size of the containing
block.

The old code is incorrect in a number of cases, such as:
1. in RTL, with 'right: 100px', it will say that the frame does not
   depend on its container's width since 'right' (offset-inline-start)
   is a fixed offset and 'left' is 'auto'.  However, since the
   positioning is relative to the right edge, a change in container size
   does require that the absolutely positioned element be repositioned
   relative to the container's left edge.
2. In vertical-rl, again with 'right: 100px', it will make the same
   mistake, since 'right' (now offset-block-start) is a fixed offset.
   This is the case from the test I was debugging.
3. In vertical-rl with rtl direction and 'bottom: 100px', we will make
   the same mistake because 'bottom' (inline-start) is fixed and 'top'
   is 'auto', and we use 'bottom' rather than 'top'.

However, in cases (1) and (3) we actually avoid hitting the bug in these
simple-ish cases because ReflowInput::ShouldReflowAllKids() returns true
whenever IsIResize() is true, which means that
nsAbsoluteContainingBlock::Reflow doesn't even call
FrameDependsOnContainer.  However, FrameDependsOnContainer should still
do the right thing because it's needed for
nsAbsoluteContainingBlock::MarkSizeDependentFramesDirty, which is only
used (from nsBlockFrame) when we reflow again for clearance or for
interruptible reflow.  I haven't attempted to write a testcase for that
because it seems likely to require spending hours in the debugger trying
to trigger the right code.

This means that the only test that fails prior to the patch is
dynamic-offset-vrl-001.html, which exercises case (2), and also happens
to be the most similar to problem in clearance-calculations-vrl-008.xht.

This patch also makes the tests stricter so that we do optimize away
resizes in some cases where we're able to do so, such as
'left: 100px; right: auto' in RTL.  (Or, rather, we would if it weren't
for the IsIResize() in ShouldReflowAllKids().)

MozReview-Commit-ID: 8xm1AHC21oh

--HG--
extra : transplant_source : %06%B4%40%EB%A9%C8M%F3%99%80%A9%DE%1F%1E%90%D3%F1%04W.
This commit is contained in:
L. David Baron 2017-03-16 09:39:19 -07:00
Родитель 8ae81abdc5
Коммит 86687429ea
9 изменённых файлов: 377 добавлений и 17 удалений

Просмотреть файл

@ -256,22 +256,6 @@ nsAbsoluteContainingBlock::FrameDependsOnContainer(nsIFrame* f,
!IsFixedMarginSize(margin->mMargin.GetIEnd(wm))) {
return true;
}
if (!wm.IsBidiLTR()) {
// Note that even if 'istart' is a length, our position can
// still depend on the containing block isze, because if
// 'iend' is also a length we will discard 'istart' and be
// positioned relative to the containing block iend edge.
// 'istart' length and 'iend' auto is the only combination
// we can be sure of.
if (!IsFixedOffset(pos->mOffset.GetIStart(wm)) ||
pos->mOffset.GetIEndUnit(wm) != eStyleUnit_Auto) {
return true;
}
} else {
if (!IsFixedOffset(pos->mOffset.GetIStart(wm))) {
return true;
}
}
}
if (wm.IsVertical() ? aCBWidthChanged : aCBHeightChanged) {
// See if f's block-size might have changed.
@ -296,10 +280,42 @@ nsAbsoluteContainingBlock::FrameDependsOnContainer(nsIFrame* f,
!IsFixedMarginSize(margin->mMargin.GetBEnd(wm))) {
return true;
}
if (!IsFixedOffset(pos->mOffset.GetBStart(wm))) {
}
// Since we store coordinates relative to top and left, the position
// of a frame depends on that of its container if it is fixed relative
// to the right or bottom, or if it is positioned using percentages
// relative to the left or top. Because of the dependency on the
// sides (left and top) that we use to store coordinates, these tests
// are easier to do using physical coordinates rather than logical.
if (aCBWidthChanged) {
if (!IsFixedOffset(pos->mOffset.GetLeft())) {
return true;
}
// Note that even if 'left' is a length, our position can still
// depend on the containing block width, because if our direction or
// writing-mode moves from right to left (in either block or inline
// progression) and 'right' is not 'auto', we will discard 'left'
// and be positioned relative to the containing block right edge.
// 'left' length and 'right' auto is the only combination we can be
// sure of.
if ((wm.GetInlineDir() == WritingMode::eInlineRTL ||
wm.GetBlockDir() == WritingMode::eBlockRL) &&
pos->mOffset.GetRightUnit() != eStyleUnit_Auto) {
return true;
}
}
if (aCBHeightChanged) {
if (!IsFixedOffset(pos->mOffset.GetTop())) {
return true;
}
// See comment above for width changes.
if (wm.GetInlineDir() == WritingMode::eInlineBTT &&
pos->mOffset.GetBottomUnit() != eStyleUnit_Auto) {
return true;
}
}
return false;
}

Просмотреть файл

@ -0,0 +1,32 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test Reference: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<style>
body {
margin: 0;
}
#container {
position: absolute;
width: 100px;
height: 100px;
background: yellow;
}
#abspos {
margin-top: 10px;
margin-left: 70px;
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>

Просмотреть файл

@ -0,0 +1,50 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-writing-modes-3/#vertical-layout">
<link rel="match" href="dynamic-offset-rtl-001-ref.html">
<meta name="flags" content="dom">
<meta name="assert" content="Layout rules for 'right' and 'left' are handled correctly for absolutely positioned elements, in the presence of dynamic changes to 'width' of the containing block.">
<style>
html {
direction: rtl;
overflow: hidden;
}
#container {
position: absolute;
top: 0;
left: 0;
width: 200px;
height: 100px;
background: yellow;
}
#abspos {
position: absolute;
top: 10px;
right: 20px;
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>
<script>
window.addEventListener("load", function(event) {
var e = document.getElementById("abspos");
e.offsetTop; // flush layout
e.parentNode.style.width = "100px";
});
</script>

Просмотреть файл

@ -0,0 +1,51 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-writing-modes-3/#vertical-layout">
<link rel="match" href="dynamic-offset-rtl-001-ref.html">
<meta name="flags" content="dom">
<meta name="assert" content="Layout rules for 'right' and 'left' are handled correctly for absolutely positioned elements, in the presence of dynamic changes to 'width' of the containing block.">
<style>
html {
direction: rtl;
overflow: hidden;
}
#container {
position: absolute;
top: 0;
left: 0;
width: 200px;
height: 100px;
background: yellow;
}
#abspos {
position: absolute;
top: 10px;
right: 20px;
left: 10px; /* ignored */
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>
<script>
window.addEventListener("load", function(event) {
var e = document.getElementById("abspos");
e.offsetTop; // flush layout
e.parentNode.style.width = "100px";
});
</script>

Просмотреть файл

@ -0,0 +1,50 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-writing-modes-3/#vertical-layout">
<link rel="match" href="dynamic-offset-rtl-001-ref.html">
<meta name="flags" content="dom">
<meta name="assert" content="Layout rules for 'right' and 'left' are handled correctly for absolutely positioned elements, in the presence of dynamic changes to 'width' of the containing block.">
<style>
html {
writing-mode: vertical-rl;
overflow: hidden;
}
#container {
position: absolute;
top: 0;
left: 0;
width: 200px;
height: 100px;
background: yellow;
}
#abspos {
position: absolute;
top: 10px;
right: 20px;
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>
<script>
window.addEventListener("load", function(event) {
var e = document.getElementById("abspos");
e.offsetTop; // flush layout
e.parentNode.style.width = "100px";
});
</script>

Просмотреть файл

@ -0,0 +1,51 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-writing-modes-3/#vertical-layout">
<link rel="match" href="dynamic-offset-rtl-001-ref.html">
<meta name="flags" content="dom">
<meta name="assert" content="Layout rules for 'right' and 'left' are handled correctly for absolutely positioned elements, in the presence of dynamic changes to 'width' of the containing block.">
<style>
html {
writing-mode: vertical-rl;
overflow: hidden;
}
#container {
position: absolute;
top: 0;
left: 0;
width: 200px;
height: 100px;
background: yellow;
}
#abspos {
position: absolute;
top: 10px;
right: 20px;
left: 10px; /* ignored */
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>
<script>
window.addEventListener("load", function(event) {
var e = document.getElementById("abspos");
e.offsetTop; // flush layout
e.parentNode.style.width = "100px";
});
</script>

Просмотреть файл

@ -0,0 +1,51 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-writing-modes-3/#vertical-layout">
<link rel="match" href="dynamic-offset-rtl-001-ref.html">
<meta name="flags" content="dom">
<meta name="assert" content="Layout rules for 'right' and 'left' are handled correctly for absolutely positioned elements, in the presence of dynamic changes to 'width' of the containing block.">
<style>
html {
writing-mode: vertical-rl;
direction: rtl;
overflow: hidden;
}
#container {
position: absolute;
top: 0;
left: 0;
width: 100px;
height: 200px;
background: yellow;
}
#abspos {
position: absolute;
bottom: 80px;
left: 70px;
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>
<script>
window.addEventListener("load", function(event) {
var e = document.getElementById("abspos");
e.offsetTop; // flush layout
e.parentNode.style.height = "100px";
});
</script>

Просмотреть файл

@ -0,0 +1,52 @@
<!DOCTYPE HTML>
<head>
<meta charset="UTF-8">
<title>CSS Test: dynamic changes to offset properties</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
<link rel="help" href="https://drafts.csswg.org/css-writing-modes-3/#vertical-layout">
<link rel="match" href="dynamic-offset-rtl-001-ref.html">
<meta name="flags" content="dom">
<meta name="assert" content="Layout rules for 'right' and 'left' are handled correctly for absolutely positioned elements, in the presence of dynamic changes to 'width' of the containing block.">
<style>
html {
writing-mode: vertical-rl;
direction: rtl;
overflow: hidden;
}
#container {
position: absolute;
top: 0;
left: 0;
width: 100px;
height: 200px;
background: yellow;
}
#abspos {
position: absolute;
bottom: 80px;
left: 70px;
top: 80px; /* ignored */
width: 10px;
height: 10px;
background: fuchsia;
}
</style>
<div id="container">
<div id="abspos"></div>
</div>
<script>
window.addEventListener("load", function(event) {
var e = document.getElementById("abspos");
e.offsetTop; // flush layout
e.parentNode.style.height = "100px";
});
</script>

Просмотреть файл

@ -15,3 +15,10 @@ default-preferences pref(layout.css.text-combine-upright.enabled,true)
== text-orientation-upright-directionality-001.html text-orientation-upright-directionality-001-ref.html
== logical-physical-mapping-001.html logical-physical-mapping-001-ref.html
== dynamic-offset-rtl-001.html dynamic-offset-rtl-001-ref.html
== dynamic-offset-rtl-002.html dynamic-offset-rtl-001-ref.html
== dynamic-offset-vrl-001.html dynamic-offset-rtl-001-ref.html
== dynamic-offset-vrl-002.html dynamic-offset-rtl-001-ref.html
== dynamic-offset-vrl-rtl-001.html dynamic-offset-rtl-001-ref.html
== dynamic-offset-vrl-rtl-002.html dynamic-offset-rtl-001-ref.html