1046101 - Redesign nsEventStructType

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

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

nsEventStructType should be generated from macro of EventClassList.h automatically.

Then, the values will be like:

KeyboardEventClass,
MouseEventClass,

Then, I think nsEventStructType should be renamed to mozilla::EventClassID and also WidgetEvent::eventStructType should be renamed to mClassID.

Any ideas?

Comment 1

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

Attachment: part.1 Rename nsEventStructType to mozilla::EventClassID

I roughly created 33 patches for this bug.
https://tbpl.mozilla.org/?tree=Try&rev=286cd58b2335
https://tbpl.mozilla.org/?tree=Try&rev=93200e41c32e

However, part.2 - part.32 are not so important they are just separated for renaming each NS_FOO_BAR_EVENT to FooBarEventClass.

So, first, part.1 and part.33 should be reviewed for preventing bug spams.

This patch renames:
nsEventStructType -> mozilla::EventClassID
WidgetEvent::eventStructType -> WidgetEvent::mClass

If you have other idea about the new names, let me know. I'll just replace them with your suggestion.

# It might be better to make mClass private and make friend class ParamTraits and create |EventClassID WidgetEvent::Class() const| for protecting the value.

Comment 2

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

Attachment: part.33 Generate EventClassID with EventClassList.h in EventForwards.h and nsContentUtils should use it instead of uint32_t

All NS_FOO_BAR_EVENT are renamed to FooBarEvent. But NS_EVENT is renamed to BasicEventClass. Perhaps, it could be GenericEventClass or SimpleEventClass too.

And it should be defined in EventForwards.h because the EventClassID should be used in the related methods of nsContentUtils but BasicEvents.h shouldn't be included by nsContentUtils.h for avoiding include hell.

Comment 3

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

Comment on attachment 8466162 [details] [diff] [review]
part.1 Rename nsEventStructType to mozilla::EventClassID

I wonder if mClass could be const.

Why you need
+    WriteParam(aMsg,
+      static_cast<mozilla::EventClassIDType>(aParam.mClass));
?

Comment 4

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

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> All NS_FOO_BAR_EVENT are renamed to FooBarEvent. But NS_EVENT is renamed to
> BasicEventClass. Perhaps, it could be GenericEventClass or SimpleEventClass
> too.
Specs talk about simple events, so perhaps SimpleEventClass

Comment 5

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

Comment on attachment 8466164 [details] [diff] [review]
part.33 Generate EventClassID with EventClassList.h in EventForwards.h and nsContentUtils should use it instead of uint32_t

But I think there is some patch missing here.
Like part 2?

Comment 6

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

(In reply to Olli Pettay [:smaug] from comment #3)
> Comment on attachment 8466162 [details] [diff] [review]
> part.1 Rename nsEventStructType to mozilla::EventClassID
> 
> I wonder if mClass could be const.

I have no idea to do that because nsGUIEventIPC.h needs default constructor. However, then, we cannot initialize mClass with proper value without calling a lot of virtual calls of AsFooEvent().

Comment 7

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

(In reply to Olli Pettay [:smaug] from comment #3)
> Why you need
> +    WriteParam(aMsg,
> +      static_cast<mozilla::EventClassIDType>(aParam.mClass));
> ?

I confirmed that if we use enum type here, we need to create struct ParamTraits<mozilla::EventClassID>. It's redundant.

Comment 8

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

Attachment: part.2 Rename NS_GUI_EVENT to GUIEventClass

Okay, let's go.

Comment 9

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

Attachment: part.3 Rename NS_INPUT_EVENT to InputEventClass

Comment 10

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

Attachment: part.4 Rename NS_UI_EVENT to UIEventClass

Comment 11

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

Attachment: part.5 Rename NS_KEY_EVENT to KeyboardEventClass

Comment 12

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

Attachment: part.6 Rename NS_COMPOSITION_EVENT to CompositionEventClass

Comment 13

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

Attachment: part.7 Rename NS_TEXT_EVENT to TextEventClass

Comment 14

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

Attachment: part.8 Rename NS_QUERY_CONTENT_EVENT to QueryContentEventClass

Comment 15

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

Attachment: part.9 Rename NS_SELECTION_EVENT to SelectionEventClass

Comment 16

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

Attachment: part.10 Rename NS_EDITOR_INPUT_EVENT to EditorInputEventClass

Comment 17

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

Attachment: part.11 Rename NS_MOUSE_EVENT to MouseEventClass

Comment 18

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

Attachment: part.12 Rename NS_DRAG_EVENT to DragEventClass

Comment 19

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

Attachment: part.13 Rename NS_MOUSE_SCROLL_EVENT to MouseScrollEventClass

Comment 20

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

Attachment: part.14 Rename NS_WHEEL_EVENT to WheelEventClass

Comment 21

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

Attachment: part.15 Rename NS_POINTER_EVENT to PointerEventClass

Comment 22

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

Attachment: part.16 Rename NS_GESTURENOTIFY_EVENT to GestureNotifyEventClass

Comment 23

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

Attachment: part.17 Rename NS_SIMPLE_GESTURE_EVENT to SimpleGestureEventClass

Comment 24

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

Attachment: part.18 Rename NS_TOUCH_EVENT to TouchEventClass

Comment 25

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

Attachment: part.19 Rename NS_SCROLLPORT_EVENT to ScrollPortEventClass

Comment 26

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

Attachment: part.20 Rename NS_SCROLLAREA_EVENT to ScrollAreaEventClass

Comment 27

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

Attachment: part.21 Rename NS_FORM_EVENT to FormEventClass

Comment 28

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

Attachment: part.22 Rename NS_FOCUS_EVENT to FocusEventClass

Comment 29

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

Attachment: part.23 Rename NS_CLIPBOARD_EVENT to ClipboardEventClass

Comment 30

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

Attachment: part.24 Rename NS_TRANSITION_EVENT to TransitionEventClass

Comment 31

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

Attachment: part.25 Rename NS_ANIMATION_EVENT to AnimationEventClass

Comment 32

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

Attachment: part.26 Rename NS_SVGZOOM_EVENT to SVGZoomEventClass

Comment 33

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

Attachment: part.27 Rename NS_SMIL_TIME_EVENT to SMILTimeEventClass

Comment 34

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

Attachment: part.28 Rename NS_COMMAND_EVENT to CommandEventClass

Comment 35

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

Attachment: part.29 Rename NS_CONTENT_COMMAND_EVENT to ContentCommandEventClass

Comment 36

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

Attachment: part.30 Rename NS_PLUGIN_COMMAND_EVENT to PluginEventClass

Comment 37

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

Attachment: part.31 Rename NS_MUTATION_EVENT to MutationEventClass

Comment 38

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

Attachment: part.32 Rename NS_EVENT to SimpleEventClass

Comment 39

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

Attachment: part.33 Generate EventClassID with EventClassList.h in EventForwards.h and nsContentUtils should use it instead of uint32_t

Comment 40

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

I actually think all the FooBarClass enum values should be
eFooBarClass. That would make the code easier to read, since it becomes clear what
FooBarClass thing is.
(And per coding style enum values should have e-prefix.)

Comment 41

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

Comment on attachment 8466604 [details] [diff] [review]
part.32 Rename NS_EVENT to SimpleEventClass

Oh, my mistake. Since we can have non-simple events using the basic struct,
we probably should call it something else. Sorry!
BaseEventClass perhaps. That fixed, r+

Comment 42

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

Comment on attachment 8466605 [details] [diff] [review]
part.33 Generate EventClassID with EventClassList.h in EventForwards.h and nsContentUtils should use it instead of uint32_t

So I think e-prefix might make code easier to read.
With that, r+.

(or feel free to explain if you don't want e-prefix)

Comment 43

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

I added e prefix. Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e08b99faf8
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0783cbdb8d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/32faf084d418
https://hg.mozilla.org/integration/mozilla-inbound/rev/17cfb4ff5a86
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e94b93c2c62
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b9d077a13aa
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e0084fdadf1
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9d1559b5320
https://hg.mozilla.org/integration/mozilla-inbound/rev/090872caa52c
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1866b96921b
https://hg.mozilla.org/integration/mozilla-inbound/rev/cddf9f550571
https://hg.mozilla.org/integration/mozilla-inbound/rev/df76962b3683
https://hg.mozilla.org/integration/mozilla-inbound/rev/955ed4eff9c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/52e95550adbf
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa613254b2f3
https://hg.mozilla.org/integration/mozilla-inbound/rev/489398167822
https://hg.mozilla.org/integration/mozilla-inbound/rev/eba87b3656a5
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf0c9fbd6cb2
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad442dc11b05
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dcfa42108aa
https://hg.mozilla.org/integration/mozilla-inbound/rev/e45296e548c4
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d500741f3e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/b55f3c2894ff
https://hg.mozilla.org/integration/mozilla-inbound/rev/8afadb6c396c
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f548e273185
https://hg.mozilla.org/integration/mozilla-inbound/rev/fca0bb35087e
https://hg.mozilla.org/integration/mozilla-inbound/rev/016c799315bb
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d873dd5eea0
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed8b6fe7bc44
https://hg.mozilla.org/integration/mozilla-inbound/rev/17473a495afb
https://hg.mozilla.org/integration/mozilla-inbound/rev/878f57134601
https://hg.mozilla.org/integration/mozilla-inbound/rev/a00fe3dedf67
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0cfff4ee625

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b6e08b99faf8
https://hg.mozilla.org/mozilla-central/rev/d0783cbdb8d8
https://hg.mozilla.org/mozilla-central/rev/32faf084d418
https://hg.mozilla.org/mozilla-central/rev/17cfb4ff5a86
https://hg.mozilla.org/mozilla-central/rev/3e94b93c2c62
https://hg.mozilla.org/mozilla-central/rev/2b9d077a13aa
https://hg.mozilla.org/mozilla-central/rev/4e0084fdadf1
https://hg.mozilla.org/mozilla-central/rev/c9d1559b5320
https://hg.mozilla.org/mozilla-central/rev/090872caa52c
https://hg.mozilla.org/mozilla-central/rev/d1866b96921b
https://hg.mozilla.org/mozilla-central/rev/cddf9f550571
https://hg.mozilla.org/mozilla-central/rev/df76962b3683
https://hg.mozilla.org/mozilla-central/rev/955ed4eff9c9
https://hg.mozilla.org/mozilla-central/rev/52e95550adbf
https://hg.mozilla.org/mozilla-central/rev/aa613254b2f3
https://hg.mozilla.org/mozilla-central/rev/489398167822
https://hg.mozilla.org/mozilla-central/rev/eba87b3656a5
https://hg.mozilla.org/mozilla-central/rev/cf0c9fbd6cb2
https://hg.mozilla.org/mozilla-central/rev/ad442dc11b05
https://hg.mozilla.org/mozilla-central/rev/0dcfa42108aa
https://hg.mozilla.org/mozilla-central/rev/e45296e548c4
https://hg.mozilla.org/mozilla-central/rev/7d500741f3e9
https://hg.mozilla.org/mozilla-central/rev/b55f3c2894ff
https://hg.mozilla.org/mozilla-central/rev/8afadb6c396c
https://hg.mozilla.org/mozilla-central/rev/6f548e273185
https://hg.mozilla.org/mozilla-central/rev/fca0bb35087e
https://hg.mozilla.org/mozilla-central/rev/016c799315bb
https://hg.mozilla.org/mozilla-central/rev/9d873dd5eea0
https://hg.mozilla.org/mozilla-central/rev/ed8b6fe7bc44
https://hg.mozilla.org/mozilla-central/rev/17473a495afb
https://hg.mozilla.org/mozilla-central/rev/878f57134601
https://hg.mozilla.org/mozilla-central/rev/a00fe3dedf67
https://hg.mozilla.org/mozilla-central/rev/c0cfff4ee625