975688 - Move all nsDOM*Event classes into mozilla::dom namespace and rename them to same as standard spec's name (i.e., nsDOM*Event -> mozilla::dom::*Event)

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement)

Description

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

If you don't disagree with the summary, please comment here as soon as possible.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I don't disagree.

Comment 2

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

Oops, I meant that you don't agree with the summary :-(

Comment 3

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

Attachment: part.1 Rename DOMWheelEvent to WheelEvent

Comment 4

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

Attachment: part.2 Rename nsDOMAnimationEvent to mozilla::dom::AnimationEvent

Comment 5

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

Attachment: part.3 Rename nsDOMBeforeUnloadEvent to mozilla::dom::BeforeUnloadEvent

Comment 6

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

Attachment: part.4 Rename nsDOMClipboardEvent to mozilla::dom::ClipboardEvent

Comment 7

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

Attachment: part.5 Rename nsDOMCommandEvent to mozilla::dom::CommandEvent

Comment 8

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

Attachment: part.6 Rename nsDOMCompositionEvent to mozilla::dom::CompositionEvent

Comment 9

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

Attachment: part.7 Rename nsDOMDataContainerEvent to mozilla::dom::DataContainerEvent

Comment 10

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

It might be better to move NS_NewDOM*Event into mozilla::dom, but anyway, we should focus to renaming nsDOM*Event for now.

Comment 11

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

Comment on attachment 8381358 [details] [diff] [review]
part.1 Rename DOMWheelEvent to WheelEvent


> 'WheelEvent': {
>-    'headerFile': 'DOMWheelEvent.h',
>-    'nativeType': 'mozilla::dom::DOMWheelEvent',
>+    'headerFile': 'WheelEvent.h',
>+    'nativeType': 'mozilla::dom::WheelEvent',
> },
I don't think we need this entry in Bindings.conf anymore, since headerFile name and nativeType are now 
the defaults.

Comment 12

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

Or, I guess you need to export WheelEvent.h to mozilla/dom so that
bindings can use mozilla/dom/WheelEvent.h

Comment 13

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

Comment on attachment 8381359 [details] [diff] [review]
part.2 Rename nsDOMAnimationEvent to mozilla::dom::AnimationEvent

Same with this, and I assume with others.
You could upload a new patch to remove stuff from Bindings.conf
and make moz.build to export headers using EXPORTS.mozilla.dom

Comment 14

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

(In reply to Olli Pettay [:smaug] from comment #12)
> Or, I guess you need to export WheelEvent.h to mozilla/dom so that
> bindings can use mozilla/dom/WheelEvent.h

(In reply to Olli Pettay [:smaug] from comment #13)
> Comment on attachment 8381359 [details] [diff] [review]
> part.2 Rename nsDOMAnimationEvent to mozilla::dom::AnimationEvent
> 
> Same with this, and I assume with others.
> You could upload a new patch to remove stuff from Bindings.conf
> and make moz.build to export headers using EXPORTS.mozilla.dom

Yes, I like this idea better too!

Comment 15

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/d047b89102aa
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb9f057658a
https://hg.mozilla.org/integration/mozilla-inbound/rev/4966af832a78
https://hg.mozilla.org/integration/mozilla-inbound/rev/6abc19b10e91
https://hg.mozilla.org/integration/mozilla-inbound/rev/7af67d41d918
https://hg.mozilla.org/integration/mozilla-inbound/rev/15320fc9ba1c
https://hg.mozilla.org/integration/mozilla-inbound/rev/e41a9f01d217

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d047b89102aa
https://hg.mozilla.org/mozilla-central/rev/8fb9f057658a
https://hg.mozilla.org/mozilla-central/rev/4966af832a78
https://hg.mozilla.org/mozilla-central/rev/6abc19b10e91
https://hg.mozilla.org/mozilla-central/rev/7af67d41d918
https://hg.mozilla.org/mozilla-central/rev/15320fc9ba1c
https://hg.mozilla.org/mozilla-central/rev/e41a9f01d217

Comment 17

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

Attachment: part.8 Rename nsDOMDeviceMotionEvent, nsDOMDeviceAcceleration and nsDOMDeviceRotationRate to mozilla::dom::DeviceMotionEvent, DeviceAcceleration and DeviceRotationRate

Comment 18

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

Attachment: part.9 Rename nsDOMDragEvent to mozilla::dom::DragEvent

Comment 19

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

Attachment: part.10 Rename nsDOMFocusEvent to mozilla::dom::FocusEvent

Comment 20

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

Attachment: part.11 Rename nsDOMKeyboardEvent to mozilla::dom::KeyboardEvent

Comment 21

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

Attachment: part.12 Rename nsDOMMessageEvent to mozilla::dom::MessageEvent

Comment 22

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

Attachment: part.13 Rename nsDOMMouseScrollEvent to mozilla::dom::MouseScrollEvent

Comment 23

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

Attachment: part.14 Rename nsDOMSimpleGestureEvent to mozilla::dom::SimpleGestureEvent

Comment 24

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

Attachment: part.15 Rename nsDOMMouseEvent to mozilla::dom::MouseEvent

Comment 25

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

Attachment: part.16 Clean up dom::PointerEvent code

Comment 26

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

Attachment: part.17 Rename dom/events/MutationEvent.h to dom/Events/InternalMutationEvent.h

Comment 27

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

Attachment: part.18 Rename nsDOMMutationEvent to mozilla::dom::MutationEvent

Comment 28

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

Comment on attachment 8382214 [details] [diff] [review]
part.16 Clean up dom::PointerEvent code

Just cleaning up the PointerEvent implementation code for using same coding style with other DOM event classes.

Comment 29

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

Comment on attachment 8382215 [details] [diff] [review]
part.17 Rename dom/events/MutationEvent.h to dom/Events/InternalMutationEvent.h

Renaming MutationEvent.h to InternalMutationEvent.h because it will conflict with renamed nsDOMMutationEvent.h.

If we move this MutationEvent.h to widget/, we can distinguish the fine as mozilla/MutationEvent.h vs mozilla/dom/MutationEvent.h. But it may cause developers confused. Therefore, I like renaming as this patch better.

Comment 30

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

Comment on attachment 8382202 [details] [diff] [review]
part.8 Rename nsDOMDeviceMotionEvent, nsDOMDeviceAcceleration and nsDOMDeviceRotationRate to mozilla::dom::DeviceMotionEvent, DeviceAcceleration and DeviceRotationRate

> 'DeviceAcceleration': {
>-    'nativeType': 'nsDOMDeviceAcceleration',
>-    'headerFile': 'nsDOMDeviceMotionEvent.h',
>+    'headerFile': 'mozilla/dom/DeviceMotionEvent.h',
>+    'nativeType': 'mozilla::dom::DeviceAcceleration',
> },
I think you don't need nativeType, since it is the default


> 
> 'DeviceMotionEvent': {
>-    'nativeType': 'nsDOMDeviceMotionEvent',
>+    'nativeType': 'mozilla::dom::DeviceMotionEvent',
> },
I think you don't need this entry in .conf at all since nativeType is now the default one.


> 
> 'DeviceRotationRate': {
>-    'nativeType': 'nsDOMDeviceRotationRate',
>-    'headerFile': 'nsDOMDeviceMotionEvent.h',
>+    'headerFile': 'mozilla/dom/DeviceMotionEvent.h',
>+    'nativeType': 'mozilla::dom::DeviceRotationRate',
> },
I think you don't need nativeType, since it is the default

Comment 31

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

Comment on attachment 8382205 [details] [diff] [review]
part.9 Rename nsDOMDragEvent to mozilla::dom::DragEvent


> 'DragEvent': {
>-    'nativeType': 'nsDOMDragEvent',
>+    'nativeType': 'mozilla::dom::DragEvent',
> },
I think you can just remove the whole 'DragEvent' entry

Comment 32

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

Comment on attachment 8382206 [details] [diff] [review]
part.10 Rename nsDOMFocusEvent to mozilla::dom::FocusEvent

> 'FocusEvent': {
>-    'nativeType': 'nsDOMFocusEvent',
>+    'nativeType': 'mozilla::dom::FocusEvent',
> },
Again, I think the whole entry is now useless.

Comment 33

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

Comment on attachment 8382208 [details] [diff] [review]
part.11 Rename nsDOMKeyboardEvent to mozilla::dom::KeyboardEvent

> 'KeyboardEvent': {
>-    'nativeType': 'nsDOMKeyboardEvent',
>+    'nativeType': 'mozilla::dom::KeyboardEvent',
> },
Same thing here

Comment 34

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

Comment on attachment 8382209 [details] [diff] [review]
part.12 Rename nsDOMMessageEvent to mozilla::dom::MessageEvent

> 'MessageEvent': {
>-    'nativeType': 'nsDOMMessageEvent',
>+    'nativeType': 'mozilla::dom::MessageEvent',
> },
again the same thing

Comment 35

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

Comment on attachment 8382210 [details] [diff] [review]
part.13 Rename nsDOMMouseScrollEvent to mozilla::dom::MouseScrollEvent

> 'MouseScrollEvent': {
>-    'nativeType': 'nsDOMMouseScrollEvent',
>+    'nativeType': 'mozilla::dom::MouseScrollEvent',
> },
Again the same thing.

Comment 36

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

Comment on attachment 8382211 [details] [diff] [review]
part.14 Rename nsDOMSimpleGestureEvent to mozilla::dom::SimpleGestureEvent


> 'SimpleGestureEvent': {
>-    'nativeType': 'nsDOMSimpleGestureEvent',
>+    'nativeType': 'mozilla::dom::SimpleGestureEvent',
> },
ditto

Comment 37

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

Comment on attachment 8382212 [details] [diff] [review]
part.15 Rename nsDOMMouseEvent to mozilla::dom::MouseEvent

> 'MouseEvent': {
>-    'nativeType': 'nsDOMMouseEvent',
>+    'nativeType': 'mozilla::dom::MouseEvent',
> },
ditto

Comment 38

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

Comment on attachment 8382216 [details] [diff] [review]
part.18 Rename nsDOMMutationEvent to mozilla::dom::MutationEvent


> 'MutationEvent': {
>-    'nativeType': 'nsDOMMutationEvent',
>+    'nativeType': 'mozilla::dom::MutationEvent',
> },
Same case as elsewhere.

Comment 39

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/39c3f1e13db0
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd1c82a9839a
https://hg.mozilla.org/integration/mozilla-inbound/rev/25906d620c44
https://hg.mozilla.org/integration/mozilla-inbound/rev/449bb2ddc4f1
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2cd59ab0a41
https://hg.mozilla.org/integration/mozilla-inbound/rev/47dd7fd95629
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9bc5bd8dc5a
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3ee0a6cbd75
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa3367ca202d
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfc7b6e52e33
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd8b08589d68

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/39c3f1e13db0
https://hg.mozilla.org/mozilla-central/rev/fd1c82a9839a
https://hg.mozilla.org/mozilla-central/rev/25906d620c44
https://hg.mozilla.org/mozilla-central/rev/449bb2ddc4f1
https://hg.mozilla.org/mozilla-central/rev/c2cd59ab0a41
https://hg.mozilla.org/mozilla-central/rev/47dd7fd95629
https://hg.mozilla.org/mozilla-central/rev/a9bc5bd8dc5a
https://hg.mozilla.org/mozilla-central/rev/b3ee0a6cbd75
https://hg.mozilla.org/mozilla-central/rev/fa3367ca202d
https://hg.mozilla.org/mozilla-central/rev/cfc7b6e52e33
https://hg.mozilla.org/mozilla-central/rev/dd8b08589d68

Comment 41

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

Attachment: part.19 Remove unnecessary items in Bindings.conf

According to your review, we can remove them now.

Comment 42

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

Attachment: part.20 Rename nsDOMTouchEvent and nsDOMTouchList to mozilla::dom::TouchEvent and TouchList

Comment 43

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

Attachment: part.21 Rename nsDOMNotifyAudioAvailableEvent to mozilla::dom::NotifyAudioAvailableEvent

Comment 44

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

Attachment: part.22 Rename nsDOMTrasitionEvent to mozilla::dom::TrasitionEvent

Comment 45

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

Attachment: part.23 Rename nsDOMXULCommandEvent to mozilla::dom::XULCommandEvent

Comment 46

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

Attachment: part.24 Rename nsDOMNotifyPaintEvent to mozilla::dom::NotifyPaintEvent

Comment 47

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

Attachment: part.25 Rename nsDOMScrollAreaEvent to mozilla::dom::ScrollAreaEvent

Comment 48

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

Attachment: part.26 Rename nsDOMUIEvent to mozilla::dom::UIEvent

Comment 49

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

Attachment: part.27 Rename nsDOMTimeEvent to mozilla::dom::TimeEvent

Although, nsDOMTimeEvent is under dom/smil, but this is the last derived event (except nsDOMTextEvent which will be removed by bug 974318) which is not in mozilla::dom namespace.

Comment 50

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

Comment on attachment 8383557 [details] [diff] [review]
part.19 Remove unnecessary items in Bindings.conf

Just make sure to push these patches to try.

Comment 51

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

(In reply to Olli Pettay [:smaug] from comment #50)
> Comment on attachment 8383557 [details] [diff] [review]
> part.19 Remove unnecessary items in Bindings.conf
> 
> Just make sure to push these patches to try.

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=aeb468ff33c8

Yeah, I'm a big fan of tryserver.

Comment 52

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

Comment on attachment 8383562 [details] [diff] [review]
part.24 Rename nsDOMNotifyPaintEvent to mozilla::dom::NotifyPaintEvent

The comments in moz.build are odd, but not your fault.

Comment 53

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

Comment on attachment 8383567 [details] [diff] [review]
part.27 Rename nsDOMTimeEvent to mozilla::dom::TimeEvent

Hmm, one could actually implement TimeEvent using Event codegenerator (either the old one, if support for nsIDOMTimeEvent is needed, or the new one if webidl only is enough), I think.

But we can do that change in a different bug.

Comment 54

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

Thank you for the quick review! I'll check the TimeEvent next week.

https://hg.mozilla.org/integration/mozilla-inbound/rev/283b3d83b335
https://hg.mozilla.org/integration/mozilla-inbound/rev/07ed9eef2248
https://hg.mozilla.org/integration/mozilla-inbound/rev/031dc7e7c074
https://hg.mozilla.org/integration/mozilla-inbound/rev/78f44c7e55f1
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb9e4644b197
https://hg.mozilla.org/integration/mozilla-inbound/rev/e711121154fb
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c632b315211
https://hg.mozilla.org/integration/mozilla-inbound/rev/5db48f8fadb9
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e1f613de77c

Comment 55

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

I'm working on renaming nsDOMEvent but it's a big patch. I'm going to post it next Monday.

It may be the last patch of this bug. However, I wonder, should I rename NS_NewDOM*Event() too? If so, perhaps, mozilla::dom::Create*Event()? Although, it might be conflict with CreateEvent() in some places.

Comment 56

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

mozilla::dom::CreateNew*Event()?

Comment 57

[Boris Zbarsky [:bzbarsky]]

If we need a factor method at all, how about mozilla::dom::New*Event ?

Comment 58

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

Sounds good to me. How about Smaug?

Comment 59

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/283b3d83b335
https://hg.mozilla.org/mozilla-central/rev/07ed9eef2248
https://hg.mozilla.org/mozilla-central/rev/031dc7e7c074
https://hg.mozilla.org/mozilla-central/rev/78f44c7e55f1
https://hg.mozilla.org/mozilla-central/rev/cb9e4644b197
https://hg.mozilla.org/mozilla-central/rev/e711121154fb
https://hg.mozilla.org/mozilla-central/rev/8c632b315211
https://hg.mozilla.org/mozilla-central/rev/5db48f8fadb9
https://hg.mozilla.org/mozilla-central/rev/4e1f613de77c

Comment 60

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

Please don't rename NS_New* since I think we should make all the events to use
similar way to construct what codegenerated events, at least, support.

FooEventInit init;
init.bubbles = true;
...

nsRefPtr<FooEvent> event = FooEvent::Construct(someTarget, NS_LITERAL_STRING("footype"), init);

Once everything uses that, we could perhaps just remove NS_New*

Comment 61

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

Okay. I'll keep them.

Comment 62

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

Attachment: part.28 Rename nsDOMEvent to mozilla::dom::Event

The last patch of this bug.

Comment 63

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b872592e441

Comment 64

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8b872592e441