Skip to content

Commit

Permalink
Bug 1665447 - Fix BoxToRect when the "relative to" frame is not an an…
Browse files Browse the repository at this point in the history
…cestor. r=dholbert,mstange

Right now the BoxToRect callback assumes that when it gets a
RECTS_ACCOUNT_FOR_TRANSFORMS flag, the "relative to" is an ancestor.

This holds for all the callers of GetAllInFlowRectsUnion except for [1],
which was introduced in bug 1581876, because they pass
GetContainingBlockForClientRect (that is, the root frame) as the root.

But that caller passes target, so IB split continuations or such would
get two siblings as the from/to frames, messing up the bounds.

An alternative would be to not pass the RECTS_ACCOUNT_FOR_TRANSFORMS
flag for that call (as it's computing a rect relative to self, but I
don't think that'd be quite correct in the presence of fragmentation
with transformed containers (if that's possible at all? would need to
think harder...)), but it seems like the API should just behave more
generally, or assert otherwise.

To that effect, this patch adds an assertion to TransformRectToAncestor
that would've caught this bug (though there are some pre-existing
violations, so we'll fix them in another bug).

[1]: https://searchfox.org/mozilla-central/rev/dfd9c0f72f9765bd4a187444e0c1e19e8834a506/dom/base/DOMIntersectionObserver.cpp#340-341

Differential Revision: https://phabricator.services.mozilla.com/D91883
  • Loading branch information
emilio committed Sep 30, 2020
1 parent 84073a0 commit c955285
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
16 changes: 14 additions & 2 deletions layout/base/nsLayoutUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3197,6 +3197,12 @@ nsRect nsLayoutUtils::TransformFrameRectToAncestor(
Maybe<Matrix4x4Flagged>* aMatrixCache /* = nullptr */,
bool aStopAtStackingContextAndDisplayPortAndOOFFrame /* = false */,
nsIFrame** aOutAncestor /* = nullptr */) {
#if 0
// FIXME(bug 1668156): This should hold.
MOZ_ASSERT(IsAncestorFrameCrossDoc(aAncestor.mFrame, aFrame),
"Fix the caller");
#endif

SVGTextFrame* text = GetContainingSVGTextFrame(aFrame);

float srcAppUnitsPerDevPixel = aFrame->PresContext()->AppUnitsPerDevPixel();
Expand Down Expand Up @@ -4565,10 +4571,12 @@ struct BoxToRect : public nsLayoutUtils::BoxCallback {
const nsIFrame* mRelativeTo;
nsLayoutUtils::RectCallback* mCallback;
uint32_t mFlags;
bool mRelativeToIsRoot;

BoxToRect(const nsIFrame* aRelativeTo, nsLayoutUtils::RectCallback* aCallback,
uint32_t aFlags)
: mRelativeTo(aRelativeTo), mCallback(aCallback), mFlags(aFlags) {}
: mRelativeTo(aRelativeTo), mCallback(aCallback), mFlags(aFlags),
mRelativeToIsRoot(!aRelativeTo->GetParent()) {}

virtual void AddBox(nsIFrame* aFrame) override {
nsRect r;
Expand All @@ -4590,7 +4598,11 @@ struct BoxToRect : public nsLayoutUtils::BoxCallback {
}
}
if (mFlags & nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS) {
r = nsLayoutUtils::TransformFrameRectToAncestor(outer, r, mRelativeTo);
if (mRelativeToIsRoot) {
r = nsLayoutUtils::TransformFrameRectToAncestor(outer, r, mRelativeTo);
} else {
nsLayoutUtils::TransformRect(outer, mRelativeTo, r);
}
} else {
r += outer->GetOffsetTo(mRelativeTo);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@
assert_equals(entries.length, 1, element.nodeName + ": Should get an entry");
assert_true(entries[0].isIntersecting, element.nodeName + ": Should be intersecting");
assert_equals(entries[0].intersectionRatio, 1, element.nodeName + ": Should be fully intersecting");

function assert_rects_equal(r1, r2, label) {
assert_equals(r1.top, r2.top, label + ": top should be equal");
assert_equals(r1.right, r2.right, label + ": right should be equal");
assert_equals(r1.bottom, r2.bottom, label + ": bottom should be equal");
assert_equals(r1.left, r2.left, label + ": left should be equal");
}

assert_rects_equal(entries[0].boundingClientRect, element.getBoundingClientRect(), element.nodeName + ": boundingClientRect should match");
assert_rects_equal(entries[0].intersectionRect, entries[0].boundingClientRect, element.nodeName + ": intersectionRect should match entry.boundingClientRect");
}
}, "IntersectionObserver on an IB split gets the right intersection ratio");
</script>

0 comments on commit c955285

Please sign in to comment.