Closed Bug 1453403 Opened 7 years ago Closed 7 years ago

Use of Ci.nsIDOMEventListener should probably go away

Categories

(Thunderbird :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 61.0

People

(Reporter: bzbarsky, Assigned: jorgk-bmo)

Details

Attachments

(1 file)

There's no reason for JS-implemented things to implement QI to nsIDOMEventListener. See bug 1453345. And it might become impossible soon anyway if I can get away with making it noscript. At that point you will start getting exceptions from uses like this.
Thanks for the heads-up. Help you please help us out here. Which usage of nsIDOMEventListener are you referring to? https://dxr.mozilla.org/comm-central/search?q=nsIDOMEventListener&redirect=false You said QI to nsIDOMEventListener. That would be this one: https://dxr.mozilla.org/comm-central/rev/3494241a0ca63ee3b224b633483e1d9a7cf954a5/mail/components/preferences/applications.js#916 Code in im/ is considered dead since Instantbird is no longer maintained. SeaMonkey/suite has a few more QI's: https://dxr.mozilla.org/comm-central/search?q=aIID.equals(Ci.nsIDOMEventListener&redirect=false
> Which usage of nsIDOMEventListener are you referring to? Any use in https://searchfox.org/comm-central/search?q=nsIDOMEventListener&case=true&path=.js or https://searchfox.org/comm-central/search?q=nsIDOMEventListener&case=true&path=.xml (or .xul or .xml but there aren't any). > That would be this one: Yes, and various others like it. Basically, Ci.nsIDOMEventListener will likely become undefined.
Hmm, so the second query https://searchfox.org/comm-central/search?q=nsIDOMEventListener&case=true&path=.xml brings up <implementation implements="nsIDOMEventListener"> Sorry, I have no idea what that does. What do we do with that?
You remove it. It's a no-op already.
More importantly, see bug 1453487. With that landed, Ci.nsIDOMEventListener is gone.
Oops, that's what I meant, the changeset I quoted is from bug 1453487, I pasted the wrong bug from comment #0. We have build with bug 1453487 merged to M-C and see no failures. Anyway, I'll do a patch now.
This should do it. Note: code in im/ is dead and I'm not doing suite/ here either, hence NI for FRG. Boris, what about: https://dxr.mozilla.org/comm-central/rev/cc738342cb1c3f4f91009fd38f762a9249d7eb39/mail/components/preferences/applications.js#947 That needs to stay, no? handleEvent() is the default name for the listener callback, right?
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Flags: needinfo?(frgrahl)
Attachment #8967488 - Flags: review?(bzbarsky)
> That needs to stay, no? The function, yes. The comment, I'd change to "EventListener" instead of "nsIDOMEventListener".
Comment on attachment 8967488 [details] [diff] [review] 1453403-nsIDOMEventListener.patch There are a bumch of uses in suite/ as well.
Attachment #8967488 - Flags: review?(bzbarsky) → review+
Thanks, Boris, I'll fix the comment before landing. Repeating comment #8: Code in im/ is dead and I'm not doing suite/ here either, hence NI for FRG.
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/80fab3583c0e Port bug 1453487: Remove needless implements='nsIDOMEventListener' and QI. r=bz
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 61.0
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/866ed0fb6e90 Follow-up: remove spurious closing parenthesis. rs=bustage-fix
Thanks Jorg. Put it in our master bug for later porting.
Flags: needinfo?(frgrahl)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: