Skip to content
Permalink
Browse files
IntersectionObserver stops tracking when cmd+ is used to zoom in
https://bugs.webkit.org/show_bug.cgi?id=241730
<rdar://92272892>

Reviewed by Alan Bujtas.

The root margin computation was broken when root margins used percentage values, and there was
effective zoom.

The existing code scaled the insets, computed via floatValueForLength(), by the zoom value. This
makes sense for fixed length insets; localRootBounds already has zoom applied, and will be compared
with element rectangles which have also taken zoom into account.

However, for percentage insets, multiplying the result of floatValueForLength() double-scales the
result (since the maximumValue is already scaled). So only multiply by zoomFactor for non-percentage
inset values.

* LayoutTests/intersection-observer/root-margin-percentage-units-with-zoom-expected.txt: Added.
* LayoutTests/intersection-observer/root-margin-percentage-units-with-zoom.html: Added.
* LayoutTests/platform/ios/TestExpectations:
* Source/WebCore/dom/Document.cpp:
(WebCore::expandRootBoundsWithRootMargin):

Canonical link: https://commits.webkit.org/251829@main
  • Loading branch information
smfr committed Jun 24, 2022
1 parent 5f6b314 commit ee62d87260e875acf59f16a621658a93f93f61d7
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 8 deletions.
@@ -0,0 +1,3 @@

PASS IntersectionObserver's root margin, with percentage units, accounts for zooming

@@ -0,0 +1,32 @@
<!DOCTYPE html>
<link rel="help" href="https://www.w3.org/TR/intersection-observer/#intersection-observer-interface">
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
#target {
position: absolute;
height: 20px;
width: 100px;
top: 41vh;
background-color: green;
}
</style>

<div id="target"></div>
<script>
if (window.internals)
window.internals.setPageZoomFactor(2);
async_test((t) => {
let options = {
rootMargin: '-40% 0px -40% 0px',
threshold: [1]
}
const target = document.getElementById("target");
let observer = new IntersectionObserver(t.step_func_done((entries) => {
assert_true(entries[0].isIntersecting, "isIntersecting");
target.remove();
}), options);
observer.observe(target);

}, "IntersectionObserver's root margin, with percentage units, accounts for zooming");
</script>
@@ -2048,6 +2048,8 @@ editing/pasteboard/paste-sanitize-crash-1.html [ Failure ]
editing/pasteboard/paste-sanitize-crash-2.html [ Failure ]
editing/pasteboard/paste-text-events.html [ Failure ]

# setPageZoomFactor() is wrong/flaky on iOS
webkit.org/b/241951 intersection-observer/root-margin-percentage-units-with-zoom.html [ Skip ]

# Local pasteboard is not implemented on iOS, so pasteboard tests used to be all disabled.

@@ -8009,14 +8009,21 @@ void Document::removeIntersectionObserver(IntersectionObserver& observer)

static void expandRootBoundsWithRootMargin(FloatRect& localRootBounds, const LengthBox& rootMargin, float zoomFactor)
{
FloatBoxExtent rootMarginFloatBox(
floatValueForLength(rootMargin.top(), localRootBounds.height()) * zoomFactor,
floatValueForLength(rootMargin.right(), localRootBounds.width()) * zoomFactor,
floatValueForLength(rootMargin.bottom(), localRootBounds.height()) * zoomFactor,
floatValueForLength(rootMargin.left(), localRootBounds.width()) * zoomFactor
);

localRootBounds.expand(rootMarginFloatBox);
auto zoomAdjustedLength = [](const Length& length, float maximumValue, float zoomFactor) {
if (length.isPercent())
return floatValueForLength(length, maximumValue);

return floatValueForLength(length, maximumValue) * zoomFactor;
};

auto rootMarginEdges = FloatBoxExtent {
zoomAdjustedLength(rootMargin.top(), localRootBounds.height(), zoomFactor),
zoomAdjustedLength(rootMargin.right(), localRootBounds.width(), zoomFactor),
zoomAdjustedLength(rootMargin.bottom(), localRootBounds.height(), zoomFactor),
zoomAdjustedLength(rootMargin.left(), localRootBounds.width(), zoomFactor)
};

localRootBounds.expand(rootMarginEdges);
}

static std::optional<LayoutRect> computeClippedRectInRootContentsSpace(const LayoutRect& rect, const RenderElement* renderer)

0 comments on commit ee62d87

Please sign in to comment.