984253 - Rename nsJSEventListener to mozilla::JSEventListener

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm,
http://mxr.mozilla.org/mozilla-central/source/dom/events/nsJSEventListener.h#19
> 19 // nsJSEventListener interface
> 20 // misnamed - JS no longer has exclusive rights over this interface!

We shouldn't change the interface name in this bug, however, we may give better name to nsEventHandler and nsJSEventListener. What are the best names for them? Any ideas?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

FYI: CVS Blame:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/events/nsJSEventListener.h&rev=1.26

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

We should rename nsJSEventListener to JSEventHandler, or some such, since it is used for event handlers. And nsIJSEventListener should be merged with that.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Olli Pettay [:smaug] from comment #3)
> And nsIJSEventListener should be merged with that.

Do you mean that nsJSEventListener should be just refcountable class and we should get rid of nsIJSEventListener?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, nsIJSEventListener is a derived interface of nsIDOMEventListener but here QIs nsIJSEventListener...
http://mxr.mozilla.org/mozilla-central/source/dom/events/EventListenerService.cpp#104

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Well, we could move IID to nsJSEventListener (which would be renamed to JSEventHandler)

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Rename nsJSEventListener.(cpp|h) to JSEventHandler.(cpp|h)

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.2 Merge nsIJSEventListener and nsJSEventListener

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.3 Rename nsEventHandler to mozilla::TypedEventHandler

I'm not sure if TypedEventHandler is good name for nsEventHandler. I believe that mozilla::EventHandler is too generic and not good name for this class.

This patch changes:

> nsEventHandler -> mozilla::TypedEventHandler
> nsEventHandler::EventHandler() -> mozilla::TypedEventHandler::NormalEventHandler()
> nsJSEventListener::GetHandler() -> nsJSEventListener::GetTypedEventHandler()
> nsJSEventListener::mHandler -> nsJSEventListener::mTypedHandler
> EventListenerManager::GetEventHandlerInternal() -> EventListenerManager::GetTypedEventHandler()

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.4 Rename nsJSEventListener to mozilla::JSEventHandler

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8399343 [details] [diff] [review]
part.2 Merge nsIJSEventListener and nsJSEventListener

+void Assign(nsISupports* aHandler, HandlerType aType) {
{ should be in the next line
(I guess you just moved this code)

+ * (Note this interface is now used to store script objects for all
+ * script languages, so is no longer JS specific)
Remove this stuff

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8399349 [details] [diff] [review]
part.4 Rename nsJSEventListener to mozilla::JSEventHandler

Oops... I forgot to request the review for this...

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8399343 [details] [diff] [review]
part.2 Merge nsIJSEventListener and nsJSEventListener

jst, could you review this as a super-reviewer?

This patch merges nsIJSEventListener and nsJSEventListener. I.e., the concrete class, nsJSEventListener becomes an interface like nsDOMEventTargetHelper.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3f4b299b81fc
https://hg.mozilla.org/integration/mozilla-inbound/rev/7992ce6151cf
https://hg.mozilla.org/integration/mozilla-inbound/rev/a37a8007adc7
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d4fce5e6a3f

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3f4b299b81fc
https://hg.mozilla.org/mozilla-central/rev/7992ce6151cf
https://hg.mozilla.org/mozilla-central/rev/a37a8007adc7
https://hg.mozilla.org/mozilla-central/rev/4d4fce5e6a3f