1171979 - Thunderbird: Change usage of draggesture to dragstart, dragdrop to drop

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Neil Deakin]

In bug 1162050, we'd like to remove support for the draggesture and dragdrop events in favour of the standard dragstart and drop events. They should work the same, so it should be a search and replace.

In addition, I'd like to remove the old nsDragAndDrop.js library and instead convert to using the standard drag and drop api. This is a more significant amount of work, so Thunderbird could include the existing version in its own code in the meantime.

Comment 1

[Kent James (:rkent)]

It would be great if someone could take on this adaptation from the Thunderbird side. Jorg? Aceman?

Comment 2

[Magnus Melin [:mkmelin]]

I can take a look.

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: bug1171979_tb_drag_events.patch

The cases I tried seem to work

Mostly automated + fix a few comments. 
 find ./mail ./mailnews ./calendar -type f -readable -writable -exec sed -i "s/draggesture/dragstart/g" {} \;
 find ./mail ./mailnews ./calendar -type f -readable -writable -exec sed -i "s/dragdrop/drop/g" {} \;

Not completely sure about the jar.mn change, havent't tried building with bug 1162050 applied yet.

Comment 4

[Magnus Melin [:mkmelin]]

The jar.mn change should be fine. At least it resolves if I map it to chrome://global/content/nsDragAndDropX.js (for testing)

Comment 5

[Neil Deakin]

Comment on attachment 8616160 [details] [diff] [review]
bug1171979_tb_drag_events.patch

That looks ok, but the intent is to remove the nsDragAndDrop.js file. I'll ensure the other bug has a patch that does this.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: bug1171979_tb_drag_events.patch

Including our own copy too, since toolkit is about to remove the file.

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8621205 [details] [diff] [review]
bug1171979_tb_drag_events.patch

LGTM, assuming you didn't change anything when copying over nsDragAndDrop.js.  ;)

Thanks, and I apologize for the length of time it took me to review.

Comment 9

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/8af06972e213 -> FIXED

Comment 10

[Magnus Melin [:mkmelin]]

Filed bug 1226362 for using the standard drag drop api.

Comment 11

[Magnus Melin [:mkmelin]]

Backed out - https://hg.mozilla.org/comm-central/rev/7c24277d79ad

ValueError: Item already in manifest: chrome/toolkit/content/global/nsDragAndDrop.js
https://treeherder.mozilla.org/logviewer.html#?job_id=26473&repo=comm-central

Comment 12

[Philip Chee]

(In reply to Magnus Melin from comment #11)

> ValueError: Item already in manifest:
> chrome/toolkit/content/global/nsDragAndDrop.js
> https://treeherder.mozilla.org/logviewer.html#?job_id=26473&repo=comm-central

SeaMonkey uses an override. See http://hg.mozilla.org/comm-central/rev/317ca58dbe02

Comment 13

[Magnus Melin [:mkmelin]]

Thx, Philip!
I must have originally tested the patch with the toolkit version eliminated. 

https://hg.mozilla.org/comm-central/rev/b0fd980cf36e -> FIXED with the manifest adjusted to use an override instead.

(try run at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=666e0912c9b3)

Comment 14

[Jorg K (CEST = GMT+2)]

Just for the record: This solution was incomplete since ondrop (eDrop) and ondragdrop (eLegacyDragDrop) are *NOT* the same. Read the regression bug 1267804 comment #13 for an analysis.