1455055 - Convert nsIDOMEventListener to taking Event, not nsIDOMEvent

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Sadly, this is going to end up as a biggish patch if it's going to compile at all...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Jorg, you'll need a comm-central version of this too.  Should be pretty straightforward once you see the changes I'm making in m-c.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Convert nsIDOMEventListener to taking an Event, not an nsIDOMEvent

This does no cleanup other than what's needed to compile.  Cleanup coming up in
later patches.

MozReview-Commit-ID: 3sOnkj71n09

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Clean up HandleEvent implementations in layout/xul

They take Event now so we can simplify some things.

MozReview-Commit-ID: HUbn7QWqRAQ

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Clean up HandleEvent implementations in layout

MozReview-Commit-ID: 7Irm8aAmeIt

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Clean up HandleEvent implementations in accessible

MozReview-Commit-ID: B3ez0ESo21g

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Clean up HandleEvent implementations in dom/xbl

MozReview-Commit-ID: 1DOlnGP4Sri

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Clean up HandleEvent implementations in dom

MozReview-Commit-ID: Ht7HQEhVS8E

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 7. Clean up remaining HandleEvent implementations

MozReview-Commit-ID: LezJYKK74H5

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8969245 [details] [diff] [review]
part 1.  Convert nsIDOMEventListener to taking an Event, not an nsIDOMEvent

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

::: dom/interfaces/events/nsIDOMEventListener.idl
@@ +4,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "domstubs.idl"
>  
> +webidl Event;

Wow, I didn't know that we can do this!

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Wow, I didn't know that we can do this!

We couldn't until two days ago.  ;)  See https://groups.google.com/d/msg/mozilla.dev.platform/uwfhoVi7r98/oypB1UrACQAJ

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8969249 [details] [diff] [review]
part 5.  Clean up HandleEvent implementations in dom/xbl

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

::: dom/xbl/nsXBLEventHandler.cpp
@@ +36,5 @@
>      return NS_ERROR_FAILURE;
>  
>    uint8_t phase = mProtoHandler->GetPhase();
>    if (phase == NS_PHASE_TARGET) {
> +    if (aEvent->EventPhase() != EventBinding::AT_TARGET)

nit: Could you wrap one line block with {} in this kind of chance?

@@ +126,5 @@
>    if (count == 0)
>      return NS_ERROR_FAILURE;
>  
>    if (mPhase == NS_PHASE_TARGET) {
> +    if (aEvent->EventPhase() != EventBinding::AT_TARGET)

nit: same here.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8969250 [details] [diff] [review]
part 6.  Clean up HandleEvent implementations in dom

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

::: dom/plugins/base/nsPluginInstanceOwner.h
@@ +99,5 @@
>  
>    // nsIDOMEventListener interfaces
>    NS_DECL_NSIDOMEVENTLISTENER
>  
> +  nsresult ProcessMouseDown(mozilla::dom::Event* aKeyEvent);

nit: aMouseEvent?

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8969251 [details] [diff] [review]
part 7.  Clean up remaining HandleEvent implementations

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

Thank you very much! These patches are really nice especially for Quantum Flow. We should keep struggling with XPCOM specific overhead.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> nit: Could you wrap one line block with {} in this kind of chance?

Done, both places.

> nit: aMouseEvent?

Done.

And yes, more XPCOM removal coming.  ;)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6567a1d09c39
part 1.  Convert nsIDOMEventListener to taking an Event, not an nsIDOMEvent.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee04ee4104b6
part 2.  Clean up HandleEvent implementations in layout/xul.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/21dc0a589533
part 3.  Clean up HandleEvent implementations in layout.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8a18e514b3d
part 4.  Clean up HandleEvent implementations in accessible.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f6f6c68e211
part 5.  Clean up HandleEvent implementations in dom/xbl.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/491b4ebcc48a
part 6.  Clean up HandleEvent implementations in dom.  r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/966c8536f1bd
part 7.  Clean up remaining HandleEvent implementations.  r=masayuki

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #10)
> > Wow, I didn't know that we can do this!
> 
> We couldn't until two days ago.  ;)  See
> https://groups.google.com/d/msg/mozilla.dev.platform/uwfhoVi7r98/oypB1UrACQAJ

Thanks!

Comment 17

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/6567a1d09c39
https://hg.mozilla.org/mozilla-central/rev/ee04ee4104b6
https://hg.mozilla.org/mozilla-central/rev/21dc0a589533
https://hg.mozilla.org/mozilla-central/rev/a8a18e514b3d
https://hg.mozilla.org/mozilla-central/rev/3f6f6c68e211
https://hg.mozilla.org/mozilla-central/rev/491b4ebcc48a
https://hg.mozilla.org/mozilla-central/rev/966c8536f1bd