1399605 - [intersection-observer] Check if target is actually being observed when unobserving

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Description

[Tobias Schneider [:tobytailor]]

We don't check if a target element passed to .observe() is actually being observed when removing it and eventually disconnecting the observer. This can lead to single observed targets being unobserved if an unobserved target is passed.

Comment 1

[Tobias Schneider [:tobytailor]]

Attachment: Check if target is actually being observed when unobserving

Comment 2

[Tobias Schneider [:tobytailor]]

Attachment: Check if target is actually being observed when unobserving

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8907767 [details] [diff] [review]
Check if target is actually being observed when unobserving

Can you add a test for this?

Comment 4

[Tobias Schneider [:tobytailor]]

Attachment: Check if target is actually being observed when unobserving. r=mrbkap

Added test.

Comment 5

[Tobias Schneider [:tobytailor]]

Attachment: Check if target is actually being observed when unobserving. r=mrbkap

Fix indentation.

Comment 6

[Tobias Schneider [:tobytailor]]

Attachment: Check if target is actually being observed when unobserving. r=mrbkap

Comment 7

[Tobias Schneider [:tobytailor]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a520dfb22aec096dfa9ca60614501e17d7ae3f1

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76e4f11cc7a6
[intersection-observer] Check if target is actually being observed when unobserving. r=mrbkap

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/76e4f11cc7a6

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8909803 [details] [diff] [review]
Check if target is actually being observed when unobserving. r=mrbkap

Review of attachment 8909803 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/test/mochitest.ini
@@ +627,1 @@
>  skip-if = toolkit == 'android'

It looks from this diff like the skip-if Android check has changed from applying to test_bug1384658.html to test_bug1399605.html. Was that intentional?

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

See comment 10.

Comment 12

[Tobias Schneider [:tobytailor]]

Ugh, totally not intentional. Kinda forgot that those statements follow a test rather then leading it. Those should really be indented to make it more clear. No excuse tho.

Comment 13

[Tobias Schneider [:tobytailor]]

Attachment: Fix wrong test being skipped

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7b3fe40c4f4
Fix wrong test being skipped. r=mrbkap

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a7b3fe40c4f4