New issue
Advanced search Search tips

Issue 910741 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 920674



Sign in to add a comment

entry.isIntersecting always returns true if threshold is 0 & observed element has an opacity style

Reported by benjvmin...@gmail.com, Nov 30

Issue description

Chrome Version       : 70.0.3538.110
OS Version: OS X 10.14.0
URLs (if applicable) : https://codepen.io/benjvmin/pen/ZmMRvP
Other browsers tested: Firefox, don't have access to Edge
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari: Intersection Observer Not Currently Supported
    Firefox: OK 
    IE/Edge: Don't have access to Edge

What steps will reproduce the problem?
1. Create new Intersection Observer instance for multiple DOM elements on an html document. Set a threshold of 0 & root of null as part of the options object passed to the Intersection Observer constructor. Make sure that there are some elements on the page outside of the root element, or viewport in this case since root is specified as null.
2. Access entry inside observer callback function and write a conditional statement that logs to the console if entry.isIntersecting returns true. Observe Elements. 
3. Add an opacity style to any of the observed elements with a value < 1.
4. Monitor when the console.log is run inside the conditional statement checking if entry.isIntersecting is true.

What is the expected result?
the console.log statement will only run when the observed element is truly intersecting with the viewport as specified by entry.isIntersecting, and regardless of any opacity styles applied to the observed element.

What happens instead of that?
entry.isIntersecting is returning true for all elements with the opacity rule < 1 applied, regardless if they are truly intersecting with the root viewport or not, and running the console.log statement immediately for each observed element where an opacity rule < 1 is specified. 

Please provide any additional information below. Attach a screenshot if
possible.

Here is Google's official Intersection Observer codepen without an opacity style applied to observed elements, network panel shows the lazy loading is working as intended as the lazy loading functionality will only run when observed elements pass the threshold of 0 and entry.isIntersecting returns true.

https://codepen.io/malchata/pen/YeMyrQ

Here is my fork of the codepen with an added opacity style to observed elements. Notice that the callback is immediately running and entry.isIntersecting is returning true, therefore logging to the console three times immediately, regardless if the element are truly intersecting with the viewport or not. Also notice that the lazy loading functionality is not working as intended since the code inside the condition checking if (entry.isIntersecting) is true runs immediately and regardless if the are actually intersecting with the viewport. the network panel will show no additional lazy loaded images.

https://codepen.io/benjvmin/pen/ZmMRvP

Please let me know if you need any additional information, Cheers.

UserAgentString: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36



 
Simplified STR: open the attached test.html
Expected: "GOOD!" is shown
Observed: three "INTERSECTED" and "BAD!" are shown

Bisected to r571637 = 3c6dd7a9a1a24a70ec20be493fe8d5355c375a1c = https://crrev.com/c/1120690 by chrishtr@chromium.org
"[IO] Use GeometryMapper unconditionally in IntersectionObserver."
Landed in 69.0.3477.0

Confirmed by force-enabling the feature in an older build (571636) via command line and observing the bug:
chrome --enable-blink-features=IntersectionObserverGeometryMapper

Note, the bug requires an iframe.
test.html
3.1 KB View Download
Labels: Needs-Triage-M70
Apologies if my steps came off as complicated, this is my first time filing a bug. Also interesting that it requires an iframe, it explains why I didn't run into this problem in my local environment, but had problems with Codepen and CodeSandbox. 
Cc: swarnasree.mukkala@chromium.org
Components: Blink
Labels: -Type-Bug -Pri-3 RegressedIn-69 Triaged-ET Target-70 Target-71 Target-72 Target-73 M-73 FoundIn-71 FoundIn-70 FoundIn-73 FoundIn-72 hasbisect OS-Linux OS-Windows Pri-1 Type-Bug-Regression
Owner: chrishtr@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on reported chrome stable #70.0.3538.110  and latest chrome version #73.0.3629.0 by using Windows 7, Ubuntu 17.10 and Mac OS 10.14 by following steps as per comment#1. 

Bisect Information:
===============
Good build: 69.0.3476.0
Bad build: 69.0.3477.0

As per comment#1 assigning to Chris Harrelson and below is the Changelog URL.
Change Log: "https://chromium.googlesource.com/chromium/src/+/3c6dd7a9a1a24a70ec20be493fe8d5355c375a1c"
Reviewed On: "https://chromium-review.googlesource.com/1120690".

@Chris: Please help us in reassigning the issue if it is not related to your change.
Thanks.!
Fix in progress.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5aeac688156a067eeac628f056e05065134fcc2e

commit 5aeac688156a067eeac628f056e05065134fcc2e
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Tue Dec 04 19:12:52 2018

[IO] Return correct non-empty value in slow path with effects.

Previously, we always returned true for "is mapped rect non-
empty?" from LocalToAncestorVisualRect in the presence of
effect nodes which may move pixels.

Now return true only if the actual computed rect is non-empty.

Bug:910741

Change-Id: I41935060706b887f58ed14a179d8975bee02c39f
Reviewed-on: https://chromium-review.googlesource.com/c/1359692
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613625}
[modify] https://crrev.com/5aeac688156a067eeac628f056e05065134fcc2e/third_party/blink/renderer/platform/graphics/paint/geometry_mapper.cc
[modify] https://crrev.com/5aeac688156a067eeac628f056e05065134fcc2e/third_party/blink/renderer/platform/graphics/paint/geometry_mapper_test.cc

Labels: ReleaseBlock-Stable Merge-Request-72
Status: Fixed (was: Assigned)
Requesting merge to 72, as it missed the deadline by only a few days
and is very low risk, and fixes an important API.
Status: Started (was: Fixed)
Labels: -Target-70 -Target-71
Components: -Blink Blink>Layout
Project Member

Comment 11 by sheriffbot@chromium.org, Dec 5

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6771ba5948d5917bce2cf1928db45cd55e382cad

commit 6771ba5948d5917bce2cf1928db45cd55e382cad
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Wed Dec 05 20:02:05 2018

[IO] Return correct non-empty value in slow path with effects.

Previously, we always returned true for "is mapped rect non-
empty?" from LocalToAncestorVisualRect in the presence of
effect nodes which may move pixels.

Now return true only if the actual computed rect is non-empty.

Bug:910741

TBR=chrishtr@chromium.org

(cherry picked from commit 5aeac688156a067eeac628f056e05065134fcc2e)

Change-Id: I41935060706b887f58ed14a179d8975bee02c39f
Reviewed-on: https://chromium-review.googlesource.com/c/1359692
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613625}
Reviewed-on: https://chromium-review.googlesource.com/c/1363954
Cr-Commit-Position: refs/branch-heads/3626@{#85}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/6771ba5948d5917bce2cf1928db45cd55e382cad/third_party/blink/renderer/platform/graphics/paint/geometry_mapper.cc
[modify] https://crrev.com/6771ba5948d5917bce2cf1928db45cd55e382cad/third_party/blink/renderer/platform/graphics/paint/geometry_mapper_test.cc

Status: Fixed (was: Started)
Thank you all for your hard work!
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/6771ba5948d5917bce2cf1928db45cd55e382cad

Commit: 6771ba5948d5917bce2cf1928db45cd55e382cad
Author: chrishtr@chromium.org
Commiter: chrishtr@chromium.org
Date: 2018-12-05 20:02:05 +0000 UTC

[IO] Return correct non-empty value in slow path with effects.

Previously, we always returned true for "is mapped rect non-
empty?" from LocalToAncestorVisualRect in the presence of
effect nodes which may move pixels.

Now return true only if the actual computed rect is non-empty.

Bug:910741

TBR=chrishtr@chromium.org

(cherry picked from commit 5aeac688156a067eeac628f056e05065134fcc2e)

Change-Id: I41935060706b887f58ed14a179d8975bee02c39f
Reviewed-on: https://chromium-review.googlesource.com/c/1359692
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613625}
Reviewed-on: https://chromium-review.googlesource.com/c/1363954
Cr-Commit-Position: refs/branch-heads/3626@{#85}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Blocking: 920674

Sign in to add a comment