Bug 243573
Summary: | IntersectionObserver intersectionRatio < 1 observed | ||
---|---|---|---|
Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ap, bfulgham, karlcow, rniwa, simon.fraser, vitor.roriz, webkit-bug-importer, zalan |
Priority: | P2 | Keywords: | BrowserCompat, GoodFirstBug, InRadar |
Version: | Safari 15 | ||
Hardware: | Unspecified | ||
OS: | Unspecified |
Ahmad Saleem
Hi Team,
Thanks for working continuously to improve Webkit better, I came across a test case while observing Chrome bug related to Intersection Observer and noted that Safari 15.6 and Safari Technical Preview 150 show IntersectionRatio below 1.
Test Case - https://jsfiddle.net/b9we0kh3/show
Chrome Bug - https://bugs.chromium.org/p/chromium/issues/detail?id=1020466
Past Safari Bug similar to this - https://bugs.webkit.org/show_bug.cgi?id=200776
*** STEPS TO REPRODUCE ***
1) Open Test case and note Intersection Ratio
2) Resize and note that it does not show "1" and always stay below "1"
<< EXPECTED RESULT >>
Like other browsers irrespective of window size, "Intersection Ratio remains 1" and do fluctuate when scrolling but it remains at "1"
<< ACTUAL RESULT >>
Safari keep Intersection Ration below 1 while all other browsers keep it at "1"
______
In case, if Safari is right and following spec then please ignore and mark this as "RESOLVED INVALID" and if it is Web-Spec (WHATWG) issue, please raise accordingly.
Thanks
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/98541023>
Simon Fraser (smfr)
This seems to happen when the element being observed has a fractional width.
Simon Fraser (smfr)
(lldb) fr va intersectionState
(WebCore::IntersectionObservationState) intersectionState = {
absoluteTargetRect = { x = 222.59375, y = 0.0, width = 958.4375, height = 167.8125 }
absoluteRootBounds = { x = 0.0, y = 0.0, width = 1389.0, height = 898.0 }
absoluteIntersectionRect = { x = 223.0, y = 0.0, width = 958.0, height = 168.0 }
isIntersecting = true
}
Note the x position of absoluteTargetRect and absoluteIntersectionRect are slightly different.
It's getting converted to an integral rect via:
LayoutRect rectInFrameViewSpace(renderer->view().frameView().contentsToView(snappedIntRect(*rectInFrameAbsoluteSpace)));
Simon Fraser (smfr)
See also https://github.com/w3c/IntersectionObserver/issues/477
Simon Fraser (smfr)
This can be fixed by using the FloatRect version of `contentsToView()` but the Chromium bug suggests there are other issues too.
Simon Fraser (smfr)
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 5d3d20c820a0e9f03642a412ad9aff493bb25ff3..5cd70b074858012e752a5f35cafaa84b97e600d0 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -8043,7 +8043,7 @@ static std::optional<LayoutRect> computeClippedRectInRootContentsSpace(const Lay
if (!intersects)
return std::nullopt;
- LayoutRect rectInFrameViewSpace(renderer->view().frameView().contentsToView(snappedIntRect(*rectInFrameAbsoluteSpace)));
+ LayoutRect rectInFrameViewSpace = LayoutRect { renderer->view().frameView().contentsToView(*rectInFrameAbsoluteSpace) };
auto* ownerRenderer = renderer->frame().ownerRenderer();
if (!ownerRenderer)
return std::nullopt;
Ahmad Saleem
(In reply to Simon Fraser (smfr) from comment #6)
> diff --git a/Source/WebCore/dom/Document.cpp
> b/Source/WebCore/dom/Document.cpp
> index
> 5d3d20c820a0e9f03642a412ad9aff493bb25ff3..
> 5cd70b074858012e752a5f35cafaa84b97e600d0 100644
> --- a/Source/WebCore/dom/Document.cpp
> +++ b/Source/WebCore/dom/Document.cpp
> @@ -8043,7 +8043,7 @@ static std::optional<LayoutRect>
> computeClippedRectInRootContentsSpace(const Lay
> if (!intersects)
> return std::nullopt;
>
> - LayoutRect
> rectInFrameViewSpace(renderer->view().frameView().
> contentsToView(snappedIntRect(*rectInFrameAbsoluteSpace)));
> + LayoutRect rectInFrameViewSpace = LayoutRect {
> renderer->view().frameView().contentsToView(*rectInFrameAbsoluteSpace) };
> auto* ownerRenderer = renderer->frame().ownerRenderer();
> if (!ownerRenderer)
> return std::nullopt;
To fix this - I need to make pull request of this change? I am not super skilled in C++ and still learning GitHub mojo in Webkit. I am happy take it offline on Webkit Slack for any guidance on how to fix this. Thanks!
Vitor Roriz
Pull request: https://github.com/WebKit/WebKit/pull/3553
Vitor Roriz
I've just noticed we have already imported tests for it and we are succeeding on both without a patch:
1. /LayoutTests/imported/w3c/web-platform-tests/intersection-observer/intersection-ratio-with-fractional-bounds.html
2. /LayoutTests/imported/w3c/web-platform-tests/intersection-observer/intersection-ratio-with-fractional-bounds-2.html
Reading the chromium thread https://bugs.chromium.org/p/chromium/issues/detail?id=1020466, for them this would happen when [a] running the test in a process-isolated cross-origin iframe, like when hosting it in fiddle, [b] regular case, out of iframe.
Their related fix is to address [b], which doesn't happen for us. While [a] happens for gecko, blink and webkit and it seems to be related to the propagation window resizing to the iframe process.
Simon Fraser (smfr)
I reproduced this locally in a test case with no frames (the JS fiddle).
EWS
Committed 254244@main (b5701774e49f): <https://commits.webkit.org/254244@main>
Reviewed commits have been landed. Closing PR #3553 and removing active labels.