Closed Bug 984253 Opened 6 years ago Closed 6 years ago

Rename nsJSEventListener to mozilla::JSEventListener

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(4 files)

No description provided.
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?
Flags: needinfo?(bugs)
We should rename nsJSEventListener to JSEventHandler, or some such, since it is used for event handlers. And nsIJSEventListener should be merged with that.
Flags: needinfo?(bugs)
(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?
Ah, nsIJSEventListener is a derived interface of nsIDOMEventListener but here QIs nsIJSEventListener...
http://mxr.mozilla.org/mozilla-central/source/dom/events/EventListenerService.cpp#104
Well, we could move IID to nsJSEventListener (which would be renamed to JSEventHandler)
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()
Attachment #8399347 - Flags: review?(bugs)
Attachment #8399340 - Flags: review?(bugs) → review+
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
Attachment #8399343 - Flags: review?(bugs) → review+
Attachment #8399347 - Flags: review?(bugs) → review+
Comment on attachment 8399349 [details] [diff] [review]
part.4 Rename nsJSEventListener to mozilla::JSEventHandler

Oops... I forgot to request the review for this...
Attachment #8399349 - Flags: review?(bugs)
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.
Attachment #8399343 - Flags: superreview?(jst)
Attachment #8399343 - Flags: superreview?(jst) → superreview+
Attachment #8399349 - Flags: review?(bugs) → review+
You need to log in before you can comment on or make changes to this bug.