1453403 - Use of Ci.nsIDOMEventListener should probably go away

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Jorg K (CEST = GMT+2)]

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 3

[Jorg K (CEST = GMT+2)]

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?

Comment 4

[Boris Zbarsky [:bzbarsky]]

You remove it.  It's a no-op already.

Comment 5

[Jorg K (CEST = GMT+2)]

See bug 1453345: https://hg.mozilla.org/mozilla-central/rev/2a9b18f3f4ac

Comment 6

[Boris Zbarsky [:bzbarsky]]

More importantly, see bug 1453487.  With that landed, Ci.nsIDOMEventListener is gone.

Comment 7

[Jorg K (CEST = GMT+2)]

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.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1453403-nsIDOMEventListener.patch

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?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> That needs to stay, no?

The function, yes.  The comment, I'd change to "EventListener" instead of "nsIDOMEventListener".

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8967488 [details] [diff] [review]
1453403-nsIDOMEventListener.patch

There are a bumch of uses in suite/ as well.

Comment 11

[Jorg K (CEST = GMT+2)]

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.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/80fab3583c0e
Port bug 1453487: Remove needless implements='nsIDOMEventListener' and QI. r=bz

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/866ed0fb6e90
Follow-up: remove spurious closing parenthesis. rs=bustage-fix

Comment 14

[Frank-Rainer Grahl (:frg)]

Thanks Jorg. Put it in our master bug for later porting.