Bug 1479859 patch 2 - Send nsChangeHint_UpdateContainingBlock when containing block-ness changes due to one property change, while another property that might trigger containing block-ness doesn't do so because of the frame type. r=emilio

This fixes a rather subtle bug.  What the underlying code here is trying
to do is remove nsChangeHint_UpdateContainingBlock when some properties
that influence whether a frame is a containing block for absolutely
positioned or fixed positioned elements have changed, but the final
calculation of being a containing block has not changed.  However, the
old code was using a function that tested whether the style could
*possibly* lead to a frame being a containing block.  Some of the
properties (like the transform properties) that lead to being a
containing block, for example, don't apply to non-replaced inlines.
Some, however, do (such as 'filter').  So if there's a dynamic change
adding or removing a filter, on an inline that also has an *ignored*
transform property (like 'transform' or 'perspective') set, then the
code prior to this patch causes us to remove the UpdateContainingBlock
hint.

This patch fixes things by testing whether being a containing block
could have changed for *any* type of frame, by separately testing the
changes.

The added tests fail without the patch and pass with the patch, viewed
in isolation.  However, without the previous patch, test 003 passes.

Test 003 also fails in Chrome (but 001 and 002 pass).

Differential Revision: https://phabricator.services.mozilla.com/D2814

--HG--
extra : rebase_source : 0a5dbb15a058cf4a43d989bf53f042c5b718e24d
This commit is contained in:
L. David Baron 2018-08-07 15:02:07 -07:00
parent 7642040d1b
commit 552f835723
7 changed files with 204 additions and 53 deletions

View File

@ -239,14 +239,23 @@ ComputedStyle::CalcStyleDifference(ComputedStyle* aNewContext,
// doesn't use Peek* functions to get the structs on the old
// context. But this isn't a big concern because these struct
// getters should be called during frame construction anyway.
if (ThreadsafeStyleDisplay()->IsAbsPosContainingBlockForAppropriateFrame(*this) ==
aNewContext->ThreadsafeStyleDisplay()->
IsAbsPosContainingBlockForAppropriateFrame(*aNewContext) &&
ThreadsafeStyleDisplay()->IsFixedPosContainingBlockForAppropriateFrame(*this) ==
aNewContext->ThreadsafeStyleDisplay()->
IsFixedPosContainingBlockForAppropriateFrame(*aNewContext)) {
const nsStyleDisplay* oldDisp = ThreadsafeStyleDisplay();
const nsStyleDisplay* newDisp = aNewContext->ThreadsafeStyleDisplay();
bool isFixedCB;
if (oldDisp->IsAbsPosContainingBlockForNonSVGTextFrames() ==
newDisp->IsAbsPosContainingBlockForNonSVGTextFrames() &&
(isFixedCB =
oldDisp->IsFixedPosContainingBlockForNonSVGTextFrames(*this)) ==
newDisp->IsFixedPosContainingBlockForNonSVGTextFrames(*aNewContext) &&
// transform-supporting frames are a subcategory of non-SVG-text
// frames, so no need to test this if isFixedCB is true (both
// before and after the change)
(isFixedCB ||
oldDisp->IsFixedPosContainingBlockForTransformSupportingFrames() ==
newDisp->IsFixedPosContainingBlockForTransformSupportingFrames())) {
// While some styles that cause the frame to be a containing block
// has changed, the overall result hasn't.
// has changed, the overall result cannot have changed (no matter
// what the frame type is).
hint &= ~nsChangeHint_UpdateContainingBlock;
}
}

View File

@ -2396,21 +2396,6 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay
// These methods are defined in nsStyleStructInlines.h.
/**
* Returns whether the element is a containing block for its
* absolutely positioned descendants.
* aContextFrame is the frame for which this is the nsStyleDisplay.
*/
inline bool IsAbsPosContainingBlock(const nsIFrame* aContextFrame) const;
/**
* The same as IsAbsPosContainingBlock, except skipping the tests that
* are based on the frame rather than the ComputedStyle (thus
* potentially returning a false positive).
*/
inline bool IsAbsPosContainingBlockForAppropriateFrame(
mozilla::ComputedStyle&) const;
/**
* Returns true when the element has the transform property
* or a related property, and supports CSS transforms.
@ -2425,6 +2410,25 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay
*/
inline bool HasPerspective(const nsIFrame* aContextFrame) const;
/**
* Returns whether the element is a containing block for its
* absolutely positioned descendants.
* aContextFrame is the frame for which this is the nsStyleDisplay.
*/
inline bool IsAbsPosContainingBlock(const nsIFrame* aContextFrame) const;
/**
* Tests for only the sub-parts of IsAbsPosContainingBlock that apply
* to nearly all frames, except those that are SVG text frames.
*
* This should be used only when the caller has the style but not the
* frame (i.e., when calculating style changes).
*
* NOTE: This (unlike IsAbsPosContainingBlock) does not include
* IsFixPosContainingBlockForNonSVGTextFrames.
*/
inline bool IsAbsPosContainingBlockForNonSVGTextFrames() const;
/**
* Returns true when the element is a containing block for its fixed-pos
* descendants.
@ -2433,12 +2437,17 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay
inline bool IsFixedPosContainingBlock(const nsIFrame* aContextFrame) const;
/**
* The same as IsFixedPosContainingBlock, except skipping the tests that
* are based on the frame rather than the ComputedStyle (thus
* potentially returning a false positive).
* Tests for only the sub-parts of IsFixedPosContainingBlock that apply
* to:
* - nearly all frames, except those that are SVG text frames.
* - frames that support CSS transforms and are not SVG text frames.
*
* This should be used only when the caller has the style but not the
* frame (i.e., when calculating style changes).
*/
inline bool IsFixedPosContainingBlockForAppropriateFrame(
inline bool IsFixedPosContainingBlockForNonSVGTextFrames(
mozilla::ComputedStyle&) const;
inline bool IsFixedPosContainingBlockForTransformSupportingFrames() const;
/**
* Returns the final combined transform.
@ -2455,9 +2464,6 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay
private:
// Helpers for above functions, which do some but not all of the tests
// for them (since transform must be tested separately for each).
inline bool HasAbsPosContainingBlockStyleInternal() const;
inline bool HasFixedPosContainingBlockStyleInternal(
mozilla::ComputedStyle&) const;
void GenerateCombinedTransform();
};

View File

@ -154,7 +154,7 @@ nsStyleDisplay::HasPerspective(const nsIFrame* aContextFrame) const
}
bool
nsStyleDisplay::HasFixedPosContainingBlockStyleInternal(
nsStyleDisplay::IsFixedPosContainingBlockForNonSVGTextFrames(
mozilla::ComputedStyle& aStyle) const
{
// NOTE: Any CSS properties that influence the output of this function
@ -173,29 +173,31 @@ nsStyleDisplay::HasFixedPosContainingBlockStyleInternal(
}
bool
nsStyleDisplay::IsFixedPosContainingBlockForAppropriateFrame(
mozilla::ComputedStyle& aStyle) const
nsStyleDisplay::IsFixedPosContainingBlockForTransformSupportingFrames() const
{
// NOTE: Any CSS properties that influence the output of this function
// should have the FIXPOS_CB flag set on them.
return HasFixedPosContainingBlockStyleInternal(aStyle) ||
HasTransformStyle() || HasPerspectiveStyle();
return HasTransformStyle() || HasPerspectiveStyle();
}
bool
nsStyleDisplay::IsFixedPosContainingBlock(const nsIFrame* aContextFrame) const
{
mozilla::ComputedStyle* style = aContextFrame->Style();
NS_ASSERTION(style->ThreadsafeStyleDisplay() == this,
"unexpected aContextFrame");
// NOTE: Any CSS properties that influence the output of this function
// should have the FIXPOS_CB flag set on them.
if (!HasFixedPosContainingBlockStyleInternal(*aContextFrame->Style()) &&
!HasTransform(aContextFrame) && !HasPerspective(aContextFrame)) {
if (!IsFixedPosContainingBlockForNonSVGTextFrames(*style) &&
(!IsFixedPosContainingBlockForTransformSupportingFrames() ||
!aContextFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms))) {
return false;
}
return !nsSVGUtils::IsInSVGTextSubtree(aContextFrame);
}
bool
nsStyleDisplay::HasAbsPosContainingBlockStyleInternal() const
nsStyleDisplay::IsAbsPosContainingBlockForNonSVGTextFrames() const
{
// NOTE: Any CSS properties that influence the output of this function
// should have the ABSPOS_CB set on them.
@ -204,28 +206,18 @@ nsStyleDisplay::HasAbsPosContainingBlockStyleInternal() const
(mWillChangeBitField & NS_STYLE_WILL_CHANGE_ABSPOS_CB);
}
bool
nsStyleDisplay::IsAbsPosContainingBlockForAppropriateFrame(
mozilla::ComputedStyle& aStyle) const
{
NS_ASSERTION(aStyle.ThreadsafeStyleDisplay() == this, "unexpected aStyle");
// NOTE: Any CSS properties that influence the output of this function
// should have the ABSPOS_CB set on them.
return HasAbsPosContainingBlockStyleInternal() ||
IsFixedPosContainingBlockForAppropriateFrame(aStyle);
}
bool
nsStyleDisplay::IsAbsPosContainingBlock(const nsIFrame* aContextFrame) const
{
NS_ASSERTION(aContextFrame->Style()->ThreadsafeStyleDisplay() == this,
mozilla::ComputedStyle *style = aContextFrame->Style();
NS_ASSERTION(style->ThreadsafeStyleDisplay() == this,
"unexpected aContextFrame");
// NOTE: Any CSS properties that influence the output of this function
// should have the ABSPOS_CB set on them.
if (!HasAbsPosContainingBlockStyleInternal() &&
!HasFixedPosContainingBlockStyleInternal(*aContextFrame->Style()) &&
!HasTransform(aContextFrame) &&
!HasPerspective(aContextFrame)) {
if (!IsAbsPosContainingBlockForNonSVGTextFrames() &&
!IsFixedPosContainingBlockForNonSVGTextFrames(*style) &&
(!IsFixedPosContainingBlockForTransformSupportingFrames() ||
!aContextFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms))) {
return false;
}
return !nsSVGUtils::IsInSVGTextSubtree(aContextFrame);

View File

@ -163693,6 +163693,30 @@
{}
]
],
"css/filter-effects/filter-cb-abspos-inline-002.html": [
[
"/css/filter-effects/filter-cb-abspos-inline-002.html",
[
[
"/css/filter-effects/filter-cb-abspos-inline-001-ref.html",
"=="
]
],
{}
]
],
"css/filter-effects/filter-cb-abspos-inline-003.html": [
[
"/css/filter-effects/filter-cb-abspos-inline-003.html",
[
[
"/css/filter-effects/filter-cb-abspos-inline-003-ref.html",
"=="
]
],
{}
]
],
"css/filter-effects/filter-contrast-001.html": [
[
"/css/filter-effects/filter-contrast-001.html",
@ -266024,6 +266048,11 @@
{}
]
],
"css/filter-effects/filter-cb-abspos-inline-003-ref.html": [
[
{}
]
],
"css/filter-effects/filter-contrast-001-ref.html": [
[
{}
@ -563629,6 +563658,18 @@
"6f99c48d5f34761ba1bc1ce7dbdfd927469ac65a",
"reftest"
],
"css/filter-effects/filter-cb-abspos-inline-002.html": [
"6fcf8fea160f5661c97be6c6f45e5f3667badf51",
"reftest"
],
"css/filter-effects/filter-cb-abspos-inline-003-ref.html": [
"8b000d96e3978b15bc08ba463ca9c139cdea7136",
"support"
],
"css/filter-effects/filter-cb-abspos-inline-003.html": [
"d050f7e810da40fd10c7243c38b0e64f02a2d581",
"reftest"
],
"css/filter-effects/filter-contrast-001-ref.html": [
"1be00e8bba72ed3203819cb51586947535ac5096",
"support"

View File

@ -0,0 +1,41 @@
<!DOCTYPE html>
<meta charset=UTF-8>
<title>CSS Filter: Establishing containing block for absolutely-positioned elements, on an inline element</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla Corporation" href="http://mozilla.com/">
<link rel="match" href="filter-cb-abspos-inline-001-ref.html">
<link rel="help" href="https://drafts.fxtf.org/filter-effects-1/#FilterProperty">
<link rel="help" href="https://drafts.fxtf.org/filter-effects-1/#supported-filter-functions">
<meta name="flags" content="dom">
<meta name="assert" content="A value other than none for the filter property results in the creation of a containing block for absolute and fixed positioned descendants unless the element it applies to is a document root element in the current browsing context.">
<meta name="assert" content="A value of 100% leaves the input unchanged.">
<style>
#cb {
perspective: 100px;
}
#abspos {
position: absolute;
top: 0;
left: 0;
width: 10px;
height: 10px;
background: blue;
}
</style>
<script>
window.addEventListener("load", function window_load(event) {
document.body.offsetTop; // flush layout
document.getElementById("cb").style.filter = "brightness(100%)";
});
</script>
<p>Filler text.</p>
<div>
<span id="cb">Blue box should cover top-left corner of this sentence.<span id="abspos"></span></span>
</div>

View File

@ -0,0 +1,23 @@
<!DOCTYPE html>
<meta charset=UTF-8>
<title>CSS Filter: Establishing containing block for absolutely-positioned elements, on an inline element</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla Corporation" href="http://mozilla.com/">
<meta name="flags" content="">
<style>
#abspos {
position: absolute;
top: 0;
right: 0;
width: 10px;
height: 10px;
background: blue;
}
</style>
<div>
<span id="cb">Blue box should be in top-right corner of screen.</span>
</div>
<div id="abspos"></div>

View File

@ -0,0 +1,39 @@
<!DOCTYPE html>
<meta charset=UTF-8>
<title>CSS Filter: Establishing containing block for absolutely-positioned elements, on an inline element</title>
<link rel="author" title="L. David Baron" href="https://dbaron.org/">
<link rel="author" title="Mozilla Corporation" href="http://mozilla.com/">
<link rel="match" href="filter-cb-abspos-inline-003-ref.html">
<link rel="help" href="https://drafts.fxtf.org/filter-effects-1/#FilterProperty">
<link rel="help" href="https://drafts.fxtf.org/filter-effects-1/#supported-filter-functions">
<meta name="flags" content="dom">
<meta name="assert" content="A value other than none for the filter property results in the creation of a containing block for absolute and fixed positioned descendants unless the element it applies to is a document root element in the current browsing context.">
<meta name="assert" content="A value of 100% leaves the input unchanged.">
<style>
#cb {
perspective: 100px;
}
#abspos {
position: absolute;
top: 0;
right: 0;
width: 10px;
height: 10px;
background: blue;
}
</style>
<script>
window.addEventListener("load", function window_load(event) {
document.body.offsetTop; // flush layout
document.getElementById("cb").style.filter = "";
});
</script>
<div>
<span id="cb" style="filter: brightness(100%)">Blue box should be in top-right corner of screen.<span id="abspos"></span></span>
</div>