1455076 - Port Bug 1455055 and bug 1455052 - Replace use of nsIDOMEvent

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1455055 +++

Boris, please adjust the summary as you deem fit. Here I've done what we always do.

Comment 1

[Jorg K (CEST = GMT+2)]

Boris, could you kindly take a look what we need to do here:
https://dxr.mozilla.org/comm-central/search?q=nsIDOMEvent&redirect=false

im/ is dead-code, so we only have
mail/test/resources/mozmill/mozmill/extension/content/stdlib/EventUtils.js
187 const masks = Ci.nsIDOMEvent;
which would need replacement when nsIDOMEvent is removed, which is about now.

Would he replacement be:
https://dxr.mozilla.org/comm-central/source/mail/test/resources/mozmill/mozmill/extension/content/stdlib/EventUtils.js#187
  if (aEvent.shiftKey)
    mval |= masks.SHIFT_MASK;
change to
    mval |= aEvent.SHIFT_MASK;

Aceman, could you take care of the EventUtils.js?

Comment 2

[Jorg K (CEST = GMT+2)]

Re-badging the bug now. Disregarding im/, we have exactly one call site:
mail/test/resources/mozmill/mozmill/extension/content/stdlib/EventUtils.js
187 const masks = Ci.nsIDOMEvent;

In suite/ I can only see nsIDOMEventListener, which we removed in bug 1453403.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So first of all, that search is ... pretty useless.  You want a search more like this:

https://searchfox.org/comm-central/search?q=nsIDOMEvent[^a-zA-Z0-9]&case=true&regexp=true&path=

Second, you may want to wait for indices to update to the changes I already landed, or do an equivalent grep locally.

Third, you can replace Ci.nsIDOMEvent with Event for purposes of getting constants off it.  You may need to Cu.importGlobalProperties(["Event"]) if you're not in window scope.

In general, looks like mail/ is set except for that mask thing (which will likely break at some point in the future when those nonstandard mask constants go away).  im/ will need changes to C++ code to use Event instead of nsIDOMEvent.  suite/ has the one use in nsTypeAheadFind.js that can use aEvent.CAPTURING_PHASE if that's a real event, or import Event and use Event.CAPTURING_PHASE.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1455076-remove-nsIDOMEvent.patch

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e3a8338a8df4
Port bug 1455052: Remove use of nsIDOMEvent and hard-code mask values. rs=bustage-fix CLOSED TREE

Comment 6

[:aceman]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #3)
> In general, looks like mail/ is set except for that mask thing (which will
> likely break at some point in the future when those nonstandard mask
> constants go away).

Thanks, so what would be the proper standard way to get the constants?

Comment 7

[Jorg K (CEST = GMT+2)]

I think there is a misunderstanding here. The "proper" way, as I understand it,  is to use
  Cu.importGlobalProperties(["Event"])
and get them from 'Event'.

I chose not to do that since we might as well hard-code them as we have done on other occasions (like Ci.nsIDOMXPathResult.NUMBER_TYPE, IIRC:
https://dxr.mozilla.org/comm-central/rev/8d636ad5810b5586433294c16cb2376d826fcca8/calendar/base/modules/utils/calXMLUtils.jsm#36

The worry is that the entire concept of those masks might disappear as Boris indicated. Also see the code here:
https://searchfox.org/mozilla-central/rev/14578d6f2d2ec5246572827061ecefa60a40855d/dom/webidl/Event.webidl#63
  // Mozilla specific legacy stuff.

Comment 8

[:aceman]

Attachment: 1455076.patch - use nsIDomWindowUtils.

What about this? Modeled after the m-c version of the function at
https://dxr.mozilla.org/comm-central/rev/5ded36cb383d3ccafd9b6c231c5120dcdae196a2/mozilla/testing/mochitest/tests/SimpleTest/EventUtils.js#335

Comment 9

[:aceman]

Comment on attachment 8969832 [details] [diff] [review]
1455076-remove-nsIDOMEvent.patch

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

OK as a quick fix, but we should try to find the proper way without hardcoding magic constants.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 8969915 [details] [diff] [review]
1455076.patch - use nsIDomWindowUtils.

I hope Boris doesn't axe nsIDOMWindowUtils next week ;-)
I think we'll wait with the check-in until we hear from him.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> Thanks, so what would be the proper standard way to get the constants?

What constants?  There is no proper standard way to get the event *_MASK constants because there is no standard reason for them to exist.  There is no information on events that relies on those constants in any way.

Note bug 674696 which tracks removing various stuff including these constants.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8969915 [details] [diff] [review]
1455076.patch - use nsIDomWindowUtils.

This seems fine, if the only point of this function is to create a modifier mask for calling nsIDOMWindowUtils.sendMouseEvent.

Comment 13

[:aceman]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #12)
> This seems fine, if the only point of this function is to create a modifier
> mask for calling nsIDOMWindowUtils.sendMouseEvent.

Yes, that is the case in mozmill. Thanks.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b6cce393a6ac
Use nsIDOMWindowUtils instead of nsIDOMEvent to get modifiers in EventUtils.js. r=jorgk