Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The intersection observation algorithm doesn't match reality #291

Open
mrbkap opened this issue Apr 11, 2018 · 1 comment
Open

The intersection observation algorithm doesn't match reality #291

mrbkap opened this issue Apr 11, 2018 · 1 comment

Comments

@mrbkap
Copy link

mrbkap commented Apr 11, 2018

While investigating a related bug in Firefox, I started looking into edge cases in the intersection observation code and found that both Firefox and Chrome's existing behavior differ from what is specified.

The problem has to do when the smallest threshold value is greater than 0. Consider this testcase. We're observing div d1. It starts not intersecting, all implementations agree that the first output should be "record[0]: false (0)". Then, as it moves to the right, the right edge of the div clips the left edge of the root element. At this point, the algorithm in the spec would have a previousThresholdIndex of 0 and previousIsIntersecting of false. Now that d1 intersects the root, isIntersecting would be true, causing us to queue an IntersectionObserverEntry. However, neither Firefox nor Chrome do this. They both treat all intersectionRatios smaller than the smallest value in observer.thresholds as equivalent.

So, my expected output would be:

record[0]: false (0)
record[0]: true (0.5)

I believe that the algorithm in the spec (assuming I'm not mistaken) would give something like:

record[0]: false (0)
record[0]: true (0)
record[0]: true (0.5)

This makes intuitive sense to me (not having studied any use cases) as I would think the smallest threshold value would be the smallest intersectionRatio that my observer would be notified about. If I cared about the edge where isIntersecting was changing, I'd make sure that I had a threshold value of 0.

I believe we should update the algorithm in the spec to match the current browsers' implementations.

One additional nitpick: step 9 in the algorithm specifies Let thresholdIndex be the index of the first entry in observer.thresholds whose value is greater than intersectionRatio, or the length of observer.thresholds if intersectionRatio is greater than or equal to the last entry in observer.thresholds. It isn't immediately clear what to do in the case where intersectionRatio is smaller than all values in observer.thresholds. I believe it would clarify matters to explicitly specify if no entry in observer.thresholds is greater than intersectionRatio, let thresholdIndex be 0.

@szager-chromium
Copy link
Collaborator

Please take a look at the pull request and let me know if it clarifies things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants