Closed Bug 1399605 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: tschneider, Assigned: tschneider)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

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.
Blocks: 1381574
Attachment #8907766 - Attachment is obsolete: true
Priority: -- → P3
Attachment #8907767 - Flags: review?(mrbkap)
Comment on attachment 8907767 [details] [diff] [review]
Check if target is actually being observed when unobserving

Can you add a test for this?
Attachment #8907767 - Flags: review?(mrbkap) → review+
Added test.
Attachment #8907767 - Attachment is obsolete: true
Fix indentation.
Attachment #8909801 - Attachment is obsolete: true
Pushed by tschneider@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76e4f11cc7a6
[intersection-observer] Check if target is actually being observed when unobserving. r=mrbkap
https://hg.mozilla.org/mozilla-central/rev/76e4f11cc7a6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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?
See comment 10.
Flags: needinfo?(tschneider)
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.
Flags: needinfo?(tschneider)
Attachment #8910068 - Flags: review?(mrbkap)
Attachment #8910068 - Flags: review?(mrbkap) → review+
Assignee: nobody → tschneider
You need to log in before you can comment on or make changes to this bug.