1446711 - Get rid of nsIDOMMouseEvent

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement, P2)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Get rid of nsIDOMMouseEvent::GetScreenX/Y

MozReview-Commit-ID: 9Y61WHTDVvF

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Get rid of nsIDOMMouseEvent::GetClientX/Y

MozReview-Commit-ID: DqMhK4wajH5

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Get rid of nsIDOMMouseEvent::GetMozInputSource

MozReview-Commit-ID: 4cwIPNPzolI

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Get rid of nsIDOMMouseEvent::GetCtrl/Shift/Alt/MetaKey

MozReview-Commit-ID: 1H2FzUHf55n

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Get rid of nsIDOMMouseEvent::GetButton

MozReview-Commit-ID: AZWzObh01uI

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Get rid of various unused nsIDOMMouseEvent bits

MozReview-Commit-ID: BmO0Ik3B8bO

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 7. Switch the nsIDOMMouseEvent::MOZ_SOURCE_* constants over to MouseEventBinding

We can't include MouseEventBinding.h in MouseEvents.h because that produces
this include loop:

MouseEventBinding.h -> UIEventBinding.h ->
nsGlobalWindow.h -> nsGlobalWindowInner.h -> nsRefreshDriver.h ->
AnimationEventDispatcher.h -> AnimationComparator.h -> Animation.h ->
EffectCompositor.h -> PseudoElementHashEntry.h -> Element.h ->
PointerEventHandler.h -> MouseEvents.h -> MouseEventBinding.h

MozReview-Commit-ID: 6FNksGil7uD

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 8. Get rid of nsIDOMMouseEvent

MozReview-Commit-ID: 2FK1MA4LGZj

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8960315 [details] [diff] [review]
part 2.   Get rid of nsIDOMMouseEvent::GetClientX/Y

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

::: dom/html/ImageDocument.cpp
@@ +640,2 @@
>        if (event) {
> +        x = event->ClientX();

nit: Might be worth simplifying the math in this block (i.e. x = event->ClientX() - img->OffsetLeft()), since it's no longer an XPCOM style getter? Up to you though.

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8960316 [details] [diff] [review]
part 3.  Get rid of nsIDOMMouseEvent::GetMozInputSource

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

::: dom/xul/nsXULPopupListener.cpp
@@ +190,5 @@
>    }
>  
>    if (mIsContext) {
>  #ifndef NS_CONTEXT_MENU_IS_MOUSEUP
> +    uint16_t inputSource = mouseEvent->MozInputSource();

nit: Do we even need the assignment anymore, or can we just use mouseEvent->MozInputSource() in the boolean check below?

Comment 11

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8960318 [details] [diff] [review]
part 5.  Get rid of nsIDOMMouseEvent::GetButton

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

::: dom/xbl/nsXBLPrototypeHandler.cpp
@@ +732,5 @@
>  {
>    if (mDetail == -1 && mMisc == 0 && (mKeyMask & cAllModifiers) == 0)
>      return true; // No filters set up. It's generic.
>  
> +  int16_t button = aMouseEvent->Button();

nit: temporary only used once, can move to check statement below

@@ +740,2 @@
>  
> +  int32_t clickcount = aMouseEvent->Detail();

nit: temporary only used once, can move to check statement below

::: extensions/spellcheck/src/mozInlineSpellChecker.cpp
@@ +1858,5 @@
>    NS_ENSURE_TRUE(mouseEvent, NS_OK);
>  
>    // ignore any errors from HandleNavigationEvent as we don't want to prevent
>    // anyone else from seeing this event.
> +  int16_t button = mouseEvent->Button();

nit: temporary only used once, can move to check statement below

::: layout/xul/nsSplitterFrame.cpp
@@ +632,2 @@
>  
> +  int16_t button = mouseEvent->Button();

nit: button isn't used after the if statement, so could probably just move getter to check below.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> nit: Might be worth simplifying the math in this block 

Good catch, fixed.

> nit: Do we even need the assignment anymore, or can we just use mouseEvent->MozInputSource() in the boolean check below?

We can do the latter, but we end up with really long lines.  And wrapping it is less readable than having the temporary.

>nit: temporary only used once, can move to check statement below
>nit: temporary only used once, can move to check statement below
>nit: temporary only used once, can move to check statement below
>nit: button isn't used after the if statement, so could probably just move getter to check below.

All fixed.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b65f0a0b2e8e
part 1.  Get rid of nsIDOMMouseEvent::GetScreenX/Y.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/4672662f16c2
part 2.   Get rid of nsIDOMMouseEvent::GetClientX/Y.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/d88f75968a29
part 3.  Get rid of nsIDOMMouseEvent::GetMozInputSource.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/3833386688c1
part 4.  Get rid of nsIDOMMouseEvent::GetCtrl/Shift/Alt/MetaKey.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f49c8b5c2d4
part 5.  Get rid of nsIDOMMouseEvent::GetButton.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e34d0184ad6
part 6.  Get rid of various unused nsIDOMMouseEvent bits.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/3073cf6d1124
part 7.  Switch the nsIDOMMouseEvent::MOZ_SOURCE_* constants over to MouseEventBinding.  r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a1e55aa1a6b
part 8.  Get rid of nsIDOMMouseEvent.  r=qdot

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b65f0a0b2e8e
https://hg.mozilla.org/mozilla-central/rev/4672662f16c2
https://hg.mozilla.org/mozilla-central/rev/d88f75968a29
https://hg.mozilla.org/mozilla-central/rev/3833386688c1
https://hg.mozilla.org/mozilla-central/rev/8f49c8b5c2d4
https://hg.mozilla.org/mozilla-central/rev/1e34d0184ad6
https://hg.mozilla.org/mozilla-central/rev/3073cf6d1124
https://hg.mozilla.org/mozilla-central/rev/7a1e55aa1a6b