Don't expose <Add/Remove>BroadcastListenerFor on XULDocument

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: bdahl, Assigned: bdahl)

Tracking

unspecified
mozilla63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
These methods are only ever used in tests and can be removed from webidl.
(Assignee)

Updated

8 months ago
(Assignee)

Comment 1

8 months ago
These methods are only ever used in tests and no longer need to be exposed.
In test_bug445177.xul I tried to preserve more of the test, but everything
after the call to addBroadcastListenerFor is dependent on that.

MozReview-Commit-ID: C4vAxNir4O8
Attachment #8998896 - Flags: review?(bzbarsky)

Updated

8 months ago
Priority: -- → P3
(Assignee)

Comment 2

8 months ago
Also, if we're fine with removing this, I'll file some comm central bugs to let them know it's going away and they can switch to just creating an <observes> elements instead.
(Assignee)

Updated

8 months ago
Component: DOM → XUL
Comment on attachment 8998896 [details] [diff] [review]
Remove <Add/Remove>BroadcastListenerFor on XULDocument webidl.

Looks like maybe we can kill off XULDocument soon....

>-    // WebIDL API

AddBroadcastListenerFor can stop being public.

Can we rejigger the test in terms of an actual element with "observes" or <observes> element?  Or is that all already tested?
Attachment #8998896 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 4

8 months ago
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] (no decent commit message means r-) from comment #3)
> Can we rejigger the test in terms of an actual element with "observes" or
> <observes> element?  Or is that all already tested?

I played a bit with this, but using an <observes> element is not a direct replacement since there is slightly different behavior between it and using the JS API. If addBroadcastListenerFor is used and the element is removed from the document, it still receives updates, whereas if you use an <observe> element and the element is removed you no longer receive updates. I'm unsure if this is by design as there's no documentation on AddBroadcastListenerFor.

It also seems to me the last part of the test is not really doing what it thinks it is doing as it never calls removeBroadcastListenerFor before it adds the "observe" attribute back. I suppose it could be testing having two observers on one element?

There are a few other tests for the <observe> element, but it doesn't look like any of them test removal and attribute syncing. Would you like me to test that?
Flags: needinfo?(bzbarsky)
I wouldn't worry about it.
Flags: needinfo?(bzbarsky)

Comment 6

8 months ago
Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/009728482717
Remove <Add/Remove>BroadcastListenerFor on XULDocument webidl. r=bz

Comment 7

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/009728482717
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.