920377 - Get rid of legacy event class names (ns*Event)

Status: RESOLVED FIXED

(Core :: Widget, defect)

Comment 1

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

Attachment: part.1 Get rid of nsMutationEvent

Comment 2

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

Attachment: part.2 Get rid of nsPluginEvent

Comment 3

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

Attachment: part.3 Get rid of nsCommandEvent

Comment 4

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

Attachment: part.1 Get rid of nsMutationEvent

Comment 5

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

Attachment: part.2 Get rid of nsPluginEvent

Comment 6

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

Attachment: part.3 Get rid of nsCommandEvent

Comment 7

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

Attachment: part.4 Get rid of nsContentCommandEvent

Comment 8

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

Attachment: part.5 Get rid of nsAnimationEvent

Comment 9

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

Attachment: part.6 Get rid of nsTransitionEvent

Comment 10

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

Attachment: part.7 Get rid of nsFocusEvent

Comment 11

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

Attachment: part.8 Get rid of nsClipboardEvent

Comment 12

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

Attachment: part.9 Get rid of nsFormEvent

Comment 13

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

Attachment: part.10 Get rid of nsScrollAreaEvent

Comment 14

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

Attachment: part.11 Get rid of nsScrollPortEvent

Comment 15

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

Attachment: part.12 Get rid of nsScriptErrorEvent

Comment 16

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

Attachment: part.13 Get rid of nsTouchEvent

Comment 17

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

Attachment: part.14 Get rid of nsSimpleGestureEvent

Comment 18

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

Attachment: part.15 Get rid of nsGestureNotifyEvent

I'll create other patches...

Comment 19

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

Comment on attachment 810489 [details] [diff] [review]
part.12 Get rid of nsScriptErrorEvent

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

::: dom/indexedDB/IDBRequest.cpp
@@ +39,5 @@
>  } // anonymous namespace
>  
>  USING_INDEXEDDB_NAMESPACE
>  using mozilla::dom::OwningIDBObjectStoreOrIDBIndexOrIDBCursor;
> +using mozilla::InternalScriptErrorEvent;

"using namespace mozilla;" instead.

Comment 20

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/b825037b2d43
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9a539f98ccc
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ba8d22427b0
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c6b7f254a3b
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fc126f540f8
https://hg.mozilla.org/integration/mozilla-inbound/rev/78f0a1cb042e
https://hg.mozilla.org/integration/mozilla-inbound/rev/99e55a7040fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/103d145e68c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/65a887bb7f11
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd82d2c60f46
https://hg.mozilla.org/integration/mozilla-inbound/rev/299f2671ffc5
https://hg.mozilla.org/integration/mozilla-inbound/rev/45f2e3fce2c1
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe2419c32ee7
https://hg.mozilla.org/integration/mozilla-inbound/rev/46cb542ac785
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbdaf3ec9aae

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b825037b2d43
https://hg.mozilla.org/mozilla-central/rev/f9a539f98ccc
https://hg.mozilla.org/mozilla-central/rev/1ba8d22427b0
https://hg.mozilla.org/mozilla-central/rev/5c6b7f254a3b
https://hg.mozilla.org/mozilla-central/rev/7fc126f540f8
https://hg.mozilla.org/mozilla-central/rev/78f0a1cb042e
https://hg.mozilla.org/mozilla-central/rev/99e55a7040fa
https://hg.mozilla.org/mozilla-central/rev/103d145e68c0
https://hg.mozilla.org/mozilla-central/rev/65a887bb7f11
https://hg.mozilla.org/mozilla-central/rev/bd82d2c60f46
https://hg.mozilla.org/mozilla-central/rev/299f2671ffc5
https://hg.mozilla.org/mozilla-central/rev/45f2e3fce2c1
https://hg.mozilla.org/mozilla-central/rev/fe2419c32ee7
https://hg.mozilla.org/mozilla-central/rev/46cb542ac785
https://hg.mozilla.org/mozilla-central/rev/bbdaf3ec9aae

Comment 22

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

Attachment: part.16 Get rid of nsAlternativeCharCode

Comment 23

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

Attachment: part.17 Get rid of nsKeyEvent

Comment 24

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

Attachment: part.18 Get rid of nsTextRangeStyle

Comment 25

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

Attachment: part.19 Get rid of nsTextRangeArray

Comment 26

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

Attachment: part.20 Get rid of nsTextRange

Comment 27

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

Attachment: part.21 Get rid of nsTextEvent

Comment 28

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

Attachment: part.22 Get rid of nsCompositionEvent

Comment 29

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

Attachment: part.23 Get rid of nsQueryContentEvent

The name, nsQueryContentEventResult, is now strange, though.

Comment 30

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

Attachment: part.24 Get rid of nsSelectionEvent

Comment 31

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

Attachment: part.25 Get rid of nsUIEvent

roc: please ignore the new XXX comment.

smaug: Could you check the XXX comment? If it's a bug or you have no idea for them for now, I'll file a bug for it. Otherwise, I'll remove the comment from this patch.

According to the DOM Events spec, I think that the activate event should be dispatched as trusted event even if it's caused by an untrusted left click event.

However, I'm not sure if DOMActivate is fired on other browsers too. If this is same behavior as other browsers, we shouldn't change it, though. Do you know this? If you don't know, I'll research other browsers' behavior.

Comment 32

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

Attachment: part.26 Get rid of nsInputEvent

roc: Please ignore the new XXX comments.

smaug: I found that some DOM event constructors don't use the most subclass for its argument. I think that we should change them with filing another bug. How about you?

Comment 33

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

Attachment: part.27 Get rid of nsMouseEvent_base

Comment 34

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

Attachment: part.28 Get rid of nsDragEvent

Comment 35

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

Attachment: part.29 Get rid of nsMouseScrollEvent

The remaining events are:

nsEvent, nsGUIEvent, nsMouseEvent and mozilla::WheelEvent. The former 3 events are used in a lot of places, therefore, I'll land them separately later.

If I rename mozilla::WheelEvent now, I'll break a patch which I'm reviewing. I'll work on this after the patch is landed (I hope it's soon).

Comment 36

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/4665d96e7f1a
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2f6e4dd2f91
https://hg.mozilla.org/integration/mozilla-inbound/rev/f94d70152968
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cef286bd24f
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e92ba62e3e6
https://hg.mozilla.org/integration/mozilla-inbound/rev/d04a5b8e1499
https://hg.mozilla.org/integration/mozilla-inbound/rev/d569f145443d
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0b1a13f856d
https://hg.mozilla.org/integration/mozilla-inbound/rev/43bc4b6c0535
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f1f90c96c91
https://hg.mozilla.org/integration/mozilla-inbound/rev/e88b33939f50
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3a3d276eb33
https://hg.mozilla.org/integration/mozilla-inbound/rev/94b92dd29c9f
https://hg.mozilla.org/integration/mozilla-inbound/rev/58a39673d3d7

landed without the new XXX comments.

Comment 37

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

Attachment: part.30 Get rid of nsGUIEvent

Comment 38

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

Attachment: part.31 Get rid of nsEvent

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4665d96e7f1a
https://hg.mozilla.org/mozilla-central/rev/d2f6e4dd2f91
https://hg.mozilla.org/mozilla-central/rev/f94d70152968
https://hg.mozilla.org/mozilla-central/rev/3cef286bd24f
https://hg.mozilla.org/mozilla-central/rev/2e92ba62e3e6
https://hg.mozilla.org/mozilla-central/rev/d04a5b8e1499
https://hg.mozilla.org/mozilla-central/rev/d569f145443d
https://hg.mozilla.org/mozilla-central/rev/d0b1a13f856d
https://hg.mozilla.org/mozilla-central/rev/43bc4b6c0535
https://hg.mozilla.org/mozilla-central/rev/5f1f90c96c91
https://hg.mozilla.org/mozilla-central/rev/e88b33939f50
https://hg.mozilla.org/mozilla-central/rev/d3a3d276eb33
https://hg.mozilla.org/mozilla-central/rev/94b92dd29c9f
https://hg.mozilla.org/mozilla-central/rev/58a39673d3d7

Comment 40

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

Attachment: part.32 Get rid of nsMouseEvent

Comment 41

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b65d2f636a6
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7b6f1ec30ee

Comment 42

[Octoploid]

/home/markus/mozilla-central/widget/gtk/nsDragService.cpp:1581:37: error: member access into incomplete type 'nsIContent'
        nsIFrame* frame = mDragPopup->GetPrimaryFrame();
                                    ^
/home/markus/mozilla-central/widget/gtk/../xpwidgets/nsBaseDragService.h:22:7: note: forward declaration of 'nsIContent'
class nsIContent;
      ^

Comment 43

[Octoploid]

diff --git a/widget/gtk/nsDragService.cpp b/widget/gtk/nsDragService.cpp
index b00b79e033ea..423dd09ca3e3 100644
--- a/widget/gtk/nsDragService.cpp
+++ b/widget/gtk/nsDragService.cpp
@@ -34,6 +34,8 @@
 #include "nsViewManager.h"
 #include "nsIFrame.h"
 #include "nsGtkUtils.h"
+#include "nsIContent.h"
+
 
 // This sets how opaque the drag image is
 #define DRAG_IMAGE_ALPHA_LEVEL 0.5

Comment 44

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/50400711c347

Comment 45

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

(In reply to Octoploid from comment #42)
> /home/markus/mozilla-central/widget/gtk/nsDragService.cpp:1581:37: error:
> member access into incomplete type 'nsIContent'
>         nsIFrame* frame = mDragPopup->GetPrimaryFrame();
>                                     ^
> /home/markus/mozilla-central/widget/gtk/../xpwidgets/nsBaseDragService.h:22:
> 7: note: forward declaration of 'nsIContent'
> class nsIContent;
>       ^

What's your environment? No environments in tinderbox don't fail to build it.

Comment 46

[Octoploid]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #45)
> (In reply to Octoploid from comment #42)
> > /home/markus/mozilla-central/widget/gtk/nsDragService.cpp:1581:37: error:
> > member access into incomplete type 'nsIContent'
> >         nsIFrame* frame = mDragPopup->GetPrimaryFrame();
> >                                     ^
> > /home/markus/mozilla-central/widget/gtk/../xpwidgets/nsBaseDragService.h:22:
> > 7: note: forward declaration of 'nsIContent'
> > class nsIContent;
> >       ^
> 
> What's your environment? No environments in tinderbox don't fail to build it.

Linux x86_64, clang trunk.

Comment 47

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2b65d2f636a6
https://hg.mozilla.org/mozilla-central/rev/f7b6f1ec30ee
https://hg.mozilla.org/mozilla-central/rev/50400711c347

Comment 48

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

Attachment: part.33 Get rid of mozilla::WheelEvent

Final patch!

Comment 49

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d275749311e

Comment 50

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0d275749311e

Comment 51

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

Comment on attachment 811612 [details] [diff] [review]
part.26 Get rid of nsInputEvent

This all has landed. Not sure feedback is needed.

Comment 52

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

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #32)
> Created attachment 811612 [details] [diff] [review]
> smaug: I found that some DOM event constructors don't use the most subclass
> for its argument. I think that we should change them with filing another
> bug. How about you?
Sounds good.

Comment 53

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

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #31)

> smaug: Could you check the XXX comment? If it's a bug or you have no idea
> for them for now, I'll file a bug for it. Otherwise, I'll remove the comment
> from this patch.
File a bug. Not sure what behavior we want there.

> However, I'm not sure if DOMActivate is fired on other browsers too. If this
> is same behavior as other browsers, we shouldn't change it, though. Do you
> know this? If you don't know, I'll research other browsers' behavior.
DOMActivate is a legacy thing from old versions of DOM Events spec.
No browser has supported it properly. We should try to remove it at some point 
(but first warn about us of it).

Comment 54

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

(In reply to Olli Pettay [:smaug] from comment #52)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #32)
> > Created attachment 811612 [details] [diff] [review]
> > smaug: I found that some DOM event constructors don't use the most subclass
> > for its argument. I think that we should change them with filing another
> > bug. How about you?
> Sounds good.

I did it at bug 920425 :-)

(In reply to Olli Pettay [:smaug] from comment #53)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #31)
> 
> > smaug: Could you check the XXX comment? If it's a bug or you have no idea
> > for them for now, I'll file a bug for it. Otherwise, I'll remove the comment
> > from this patch.
> File a bug. Not sure what behavior we want there.

Bug 930843

> > However, I'm not sure if DOMActivate is fired on other browsers too. If this
> > is same behavior as other browsers, we shouldn't change it, though. Do you
> > know this? If you don't know, I'll research other browsers' behavior.
> DOMActivate is a legacy thing from old versions of DOM Events spec.
> No browser has supported it properly. We should try to remove it at some
> point 
> (but first warn about us of it).

Hmm, I'm afraid to remove it... Especially, I think that this event doesn't harm performance.