812427 - Sort out event struct types in nsGUIEvent.h

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

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

Looks like some struct types defined in nsGUIEvent.h isn't being used. We can remove it.

And I think that for safety, we should change them to typed enum.

Comment 1

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

Attachment: part.1 Remove unused event struct type

Comment 2

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

Attachment: part.2 Make event struct type named enumeration

This change enables build time check if the struct type is used for other purpose.

NS_POPUP_EVENT will be removed by next patch.

I'm not sure why there are the SVG event types.

Comment 3

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

Attachment: part.3 Remove NS_POPUP_EVENT

Comment 4

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

Comment on attachment 683041 [details] [diff] [review]
part.2 Make event struct type named enumeration

I would keep nsEventStructType in nsGUIEvent.h since that is where (almost)
all the event structs are.
That fixed, r=me

Comment 5

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

(In reply to Olli Pettay [:smaug] from comment #4)
> I would keep nsEventStructType in nsGUIEvent.h since that is where (almost)
> all the event structs are.
> That fixed, r=me

Hmm, nsEvent.h is included by nsGUIEvent.h. If some *.h files need nsEventStructType for arguments or members, including nsEvent.h is better since nsGUIEvent.h includes a lot of *.h files for implementing the all event structs.

Roc, how do you think about that?
# If we make nsGUIEvent.cpp, we don't need the separated header files though.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 683041 [details] [diff] [review]
part.2 Make event struct type named enumeration

Review of attachment 683041 [details] [diff] [review]:
-----------------------------------------------------------------

What Olli said.

Comment 7

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

Comment on attachment 683042 [details] [diff] [review]
part.3 Remove NS_POPUP_EVENT

I wonder when NS_POPUP_EVENT was still used, and for what.
Must be something really ancient, since even 1.8.0 doesn't seem to use it.

Comment 8

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

Okay, I'll move the enum to nsGUIEvent.h and land them tomorrow. Thank you roc and smaug!

Comment 9

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

I meant it's after the merging.

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/4fddad78629a
https://hg.mozilla.org/integration/mozilla-inbound/rev/da4c2191ba17
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d5bb3935ef9

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4fddad78629a
https://hg.mozilla.org/mozilla-central/rev/da4c2191ba17
https://hg.mozilla.org/mozilla-central/rev/9d5bb3935ef9