Bug 1316101 - Avoid excessive clamping in StickyScrollContainer::GetScrollRanges(). r=mstange
Excessive clamping can cause incorrect behaviour in the presence of negative
margins.
MozReview-Commit-ID: AkQEqcQpAxx
--- a/layout/generic/StickyScrollContainer.cpp
+++ b/layout/generic/StickyScrollContainer.cpp
@@ -278,26 +278,17 @@ StickyScrollContainer::GetScrollRanges(n
nsRect stick;
nsRect contain;
ComputeStickyLimits(firstCont, &stick, &contain);
aOuter->SetRect(nscoord_MIN/2, nscoord_MIN/2, nscoord_MAX, nscoord_MAX);
aInner->SetRect(nscoord_MIN/2, nscoord_MIN/2, nscoord_MAX, nscoord_MAX);
- // Due to margin collapsing, |firstCont->GetNormalPosition()| can sometimes
- // fall outside of |contain|. (This is because GetNormalPosition() returns
- // the actual position after margin collapsing, while|contain| is
- // calculated based on the frame's GetUsedMargin() which is pre-collapsing.)
- // This can cause |aInner|, as computed below, to not be contained inside
- // |aOuter|, which confuses the code that consumes these values.
- // This is hard to fix properly (TODO), but clamping |normalPosition| to
- // |contain| works around it.
- const nsPoint normalPosition =
- contain.ClampPoint(firstCont->GetNormalPosition());
+ const nsPoint normalPosition = firstCont->GetNormalPosition();
// Bottom and top
if (stick.YMost() != nscoord_MAX/2) {
aOuter->SetTopEdge(contain.y - stick.YMost());
aInner->SetTopEdge(normalPosition.y - stick.YMost());
}
if (stick.y != nscoord_MIN/2) {
@@ -310,16 +301,27 @@ StickyScrollContainer::GetScrollRanges(n
aOuter->SetLeftEdge(contain.x - stick.XMost());
aInner->SetLeftEdge(normalPosition.x - stick.XMost());
}
if (stick.x != nscoord_MIN/2) {
aInner->SetRightEdge(normalPosition.x - stick.x);
aOuter->SetRightEdge(contain.XMost() - stick.x);
}
+
+ // Make sure |inner| does not extend outside of |outer|. (The consumers of
+ // the Layers API, to which this information is propagated, expect this
+ // invariant to hold.) The calculated value of |inner| can sometimes extend
+ // outside of |outer|, for example due to margin collapsing, since
+ // GetNormalPosition() returns the actual position after margin collapsing,
+ // while |contain| is calculated based on the frame's GetUsedMargin() which
+ // is pre-collapsing.
+ // Note that this doesn't necessarily solve all problems stemming from
+ // comparing pre- and post-collapsing margins (TODO: find a proper solution).
+ *aInner = aInner->Intersect(*aOuter);
}
void
StickyScrollContainer::PositionContinuations(nsIFrame* aFrame)
{
NS_ASSERTION(nsLayoutUtils::IsFirstContinuationOrIBSplitSibling(aFrame),
"Should be starting from the first continuation");
nsPoint translation = ComputePosition(aFrame) - aFrame->GetNormalPosition();
--- a/layout/reftests/async-scrolling/reftest.list
+++ b/layout/reftests/async-scrolling/reftest.list
@@ -21,16 +21,17 @@ skip-if(!asyncPan) == position-fixed-cov
skip-if(!asyncPan) == position-fixed-cover-2.html position-fixed-cover-2-ref.html
skip-if(!asyncPan) == position-fixed-cover-3.html position-fixed-cover-3-ref.html
fuzzy-if(Android,5,4) skip-if(!asyncPan) == position-fixed-transformed-1.html position-fixed-transformed-1-ref.html
skip-if(!asyncPan) == split-layers-1.html split-layers-1-ref.html
skip-if(!asyncPan) == split-layers-multi-scrolling-1.html split-layers-multi-scrolling-1-ref.html
fuzzy-if(skiaContent,2,240000) fuzzy-if(browserIsRemote&&!skiaContent&&(cocoaWidget||winWidget),1,240000) skip-if(!asyncPan) == split-opacity-layers-1.html split-opacity-layers-1-ref.html
skip-if(!asyncPan) == sticky-pos-scrollable-1.html sticky-pos-scrollable-1-ref.html
skip-if(!asyncPan) == sticky-pos-scrollable-2.html sticky-pos-scrollable-2-ref.html
+skip-if(!asyncPan) == sticky-pos-scrollable-3.html sticky-pos-scrollable-3-ref.html
skip-if(!asyncPan) == fixed-pos-scrollable-1.html fixed-pos-scrollable-1-ref.html
skip-if(!asyncPan) == culling-1.html culling-1-ref.html
skip-if(!asyncPan) == position-fixed-iframe-1.html position-fixed-iframe-1-ref.html
skip-if(!asyncPan) == position-fixed-iframe-2.html position-fixed-iframe-2-ref.html
fuzzy-if(skiaContent,1,11300) skip-if(!asyncPan) == position-fixed-in-scroll-container.html position-fixed-in-scroll-container-ref.html
skip-if(!asyncPan) == position-fixed-inside-sticky-1.html position-fixed-inside-sticky-1-ref.html
skip-if(!asyncPan) == position-fixed-inside-sticky-2.html position-fixed-inside-sticky-2-ref.html
fuzzy(1,60000) skip-if(!asyncPan) == group-opacity-surface-size-1.html group-opacity-surface-size-1-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/async-scrolling/sticky-pos-scrollable-3-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+ <style>
+ #header {
+ position: fixed;
+ top: 0;
+ }
+ #header > div {
+ border: solid blue 2px;
+ height: 100px;
+ width: 100px;
+ }
+ </style>
+ <div id="section">
+ <div id="header">
+ <div></div>
+ </div>
+ </div>
+</html>
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/layout/reftests/async-scrolling/sticky-pos-scrollable-3.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html reftest-async-scroll
+ reftest-displayport-x="0" reftest-displayport-y="0"
+ reftest-displayport-w="800" reftest-displayport-h="2000"
+ reftest-async-scroll-x="0" reftest-async-scroll-y="100">
+ <style>
+ html {
+ overflow: hidden;
+ }
+ #section {
+ padding-top: 1px;
+ }
+ #header {
+ position: sticky;
+ top: 0;
+ }
+ #header > div {
+ margin-top: -50px;
+ border: solid blue 2px;
+ height: 100px;
+ width: 100px;
+ }
+ #spacer {
+ height: 1500px;
+ }
+ </style>
+ <div id="section">
+ <div id="header">
+ <div></div>
+ </div>
+ <div id="spacer"></div>
+ </div>
+</html>
\ No newline at end of file