Closed Bug 1696489 Opened 3 years ago Closed 3 years ago

Implement tests to cover the new attachment Drag and Drop workflow in the compose window

Categories

(Thunderbird :: Message Compose Window, task)

Tracking

(thunderbird_esr78 wontfix)

RESOLVED FIXED
89 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: aleca, Assigned: aleca)

References

Details

Attachments

(1 file, 2 obsolete files)

The new UI handling drag and drop of attachments needs to be covered by tests due to its nature.
These are the things that should be covered by the test:

  • Test if the overlay appears and disappears properly.
  • Test if the "Add as attachment" and "Insert inline" bucket are properly shown and hidden based on the file type.
  • Test is the file is properly handled based on the dropped target.
  • Test that the Escape key always hides the attachment overlay.
Keywords: testcase
Attached patch 1696489-attachment-test.diff (obsolete) — Splinter Review

Asking a little feedback here to see if I'm doing the right thing and if what I'm trying to do is possible.
I want to simulate the drag over movement of an external file above the compose editor and wait for the overlay to appear.

That doesn't seem to happen and I'm not sure what I'm doing wrong.

Attachment #9207355 - Flags: feedback?(geoff)

Comment on attachment 9207355 [details] [diff] [review]
1696489-attachment-test.diff

Did you succeed in swapping text/x-moz-url for application/x-moz-file (as seen here)?

Attachment #9207355 - Flags: feedback?(geoff) → feedback+

No success unfortunately.
I tried different combinations and copied what m-c did in various other tests, but nothing seems to work. The overlay simply doesn't appear (but of course it does if I manually drag a file over the compose window while the test is running).
I'm a bit lost.

Attached patch 1696489-attachment-test.diff (obsolete) — Splinter Review

This escapes me as I can't understand why the drag action is not working properly.

I think it might be related to the fact that we're passing the same element as both source and destination to the EventUtils.synthesizeDragOver() method, like here: https://searchfox.org/mozilla-central/rev/eed391546532eeafb0c1d17de0610d6cc9e65ca6/toolkit/mozapps/extensions/test/browser/browser_dragdrop.js#137-138

But I'm not sure.
Any idea what I'm doing wrong?

Attachment #9207355 - Attachment is obsolete: true
Attachment #9210569 - Flags: feedback?(mkmelin+mozilla)
Attachment #9210569 - Flags: feedback?(geoff)

Our code is bailing out here at isDataFlavorSupported which is in turn bailing out here, at least on Linux. I don't quite understand why.

On Mac it throws an exception in isDataFlavorSupported instead of returning false, but same effect. (I guess you knew that from the log, but there's no such log message on Linux, so I didn't.)

Comment on attachment 9210569 [details] [diff] [review]
1696489-attachment-test.diff

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

Maybe what Geoff noted can help...

::: mail/test/browser/composition/browser_attachmentDragDrop.js
@@ +89,5 @@
> +  return session;
> +}
> +
> +/**
> + * Helper method to simualte a darg and drop action above the window.

typo
Attachment #9210569 - Flags: feedback?(mkmelin+mozilla)

(In reply to Geoff Lankow (:darktrojan) from comment #5)

Our code is bailing out here at isDataFlavorSupported which is in turn bailing out here, at least on Linux. I don't quite understand why.

Removing the for loop and replacing it with a condition that check the event.dataTransfer.types instead of the dragSession makes the test work.
It doesn't seem to affect the functionality of the whole section, so I think we could replace it.
Any concerns about it?

I'll write the test to cover all the scenarios and also to be sure that dragging pills or elements within the editor doesn't create any unexpected results.

Status: NEW → ASSIGNED
Attachment #9210569 - Flags: feedback?(geoff)

The previous try run was mostly green, except for some tests which expected the pop account that is configured on the first test.
So, my new test shouldn't delete the default account on teardownModule.
New try run: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=c5bbe936098dd855e87e519d6662c3203ef3f0ea

Attachment #9210569 - Attachment is obsolete: true
Target Milestone: --- → 89 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/745fc0139eff
Implement tests to cover the new attachment Drag and Drop workflow in the compose window. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: