Closed Bug 1268474 Opened 8 years ago Closed 8 years ago

Drag+drop of messages from threadpane to button broken

Categories

(Thunderbird :: Folder and Message Lists, defect)

Product:
Thunderbird
Component:
Folder and Message Lists
Version:
48 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INVALID

People

(Reporter: realRaven, Unassigned)

References

Details

(Whiteboard: [addon:QuickFolders])

Attachments

(1 file)

QuickFolders prerelease for testing
693.42 KB, application/x-xpinstall
Details 
My addon uses drag+drop for dropping mails to toolbarbuttons. This was reported as broken in Thunderbird Daily 48.0a2.

In the drop event, when I am running nsITransferable.getAnyTransferData in order to pull out the flavor "text/x-moz-message" it will now throw:

Exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsITransferable.getAnyTransferData]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://quickfolders/content/quickfolders.js :: btnObs_onDrop :: line 1550"  data: no]

Interestingly,  dragging emails to the folder tree is still working.
Why does this depend on bug 270292? I think the culprit you're looking for is bug 1267259. This has landed on inbound and should be merged to M-C any minute now. Uplift to Aurora (mozilla48) is requested. So the next Daily (TB 49) should be fine, or the Earlybird (TB 48) built after the Aurora uplift.

Please keep an eye on it.
No longer depends on: 270292
For creating a quick test case: 
1 - install the current version of QuickFolders
2 - drag any folder (not the inbox) to the new toolbar - this will create a "tab" button
3 - drag any email to the "tab"

expected behavior: the email is moved to the related folder URI

observer behavior: the addon code throws when calling nsITransferable.getAnyTransferData in the drop observer
(In reply to Jorg K (GMT+2) from comment #1)
> Why does this depend on bug 270292? I think the culprit you're looking for
> is bug 1267259. This has landed on inbound and should be merged to M-C any
> minute now. Uplift to Aurora (mozilla48) is requested. So the next Daily (TB
> 49) should be fine, or the Earlybird (TB 48) built after the Aurora uplift.
> 
> Please keep an eye on it.

I was looking at hg-blame and recent changes to the ThreadPaneOnDragStart as I assumed this would be the culprit.

http://hg.mozilla.org/comm-central/filelog/0e1299ac02cc/mail/base/content/msgMail3PaneWindow.js
Whiteboard: [regression:TB48]
See Also: → 1267804
Is that working now that bug 1267259 has landed?
No it doesn't.

In debug mode I get a crash on nsTransferable.cpp:444.

You need to look into your plugin and see what it does.
Please fix your add-on or let us know if you need a hand with that.
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: regression
Resolution: --- → INVALID
Whiteboard: [regression:TB48] → [addon:QuickFolders]
Arguably those should not be  MOZ_ASSERT(mInitialized); but something like NS_ENSURE_TRUE(mInitialized, NS_ERROR_FAILURE); though
Looking more closely the mInitialized is ifdef DEBUG so an assertion is appropriate for checking mInitialized.
That said, there probably should be some non-debug checks that prevent crashes.
(In reply to Jorg K (PTO during summer, NI me) from comment #6)
> Please fix your add-on or let us know if you need a hand with that.

HI guys. sorry only just saw that you set the bug to INVALID. What exactly do I need to do to fix it? How can it be invalid when it worked in the last 80 versions - and it works fine in the beta and release version.

I just got another bug report from a Daily 64bit user:

" This happens on both my desktop running Windows 10 64-bit and the latest 64-bit Daily, and on my laptop running Vista Home 32-bit and the latest 32-bit Daily "
I am having trouble connecting the debugger to Earlybird but I have some debug / log routines in QuickFolders which can be activated via its advanced options menu. (Debug checkbox and additional information on Drag + Drop via right-click and setting 

extensions.quickfolders.debug.dnd = true
)

The logs I am getting:

QuickFolders {DND} 9:8:12.424  [8910 ms]	 
buttonDragObserver.onDragEnter - sourceNode = treechildren
  ALT = false  CTRL = false  SHIFT = false
 ----------
QuickFolders {DND} 9:8:12.867  [443 ms]	 
buttonDragObserver.onDrop flavor=text/x-moz-message
 ----------
QuickFolders:Exception in onDrop item 0 of 1
Exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsITransferable.getAnyTransferData]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://quickfolders/content/quickfolders.js :: btnObs_onDrop :: line 1550"  data: no]
 ----------
Mon May 09 2016 09:08:12
Warning: ReferenceError: reference to undefined property messageUris[0]
Source file: chrome://quickfolders/content/quickfolders.js
Line: 1571
 ----------
Mon May 09 2016 09:08:12
Error: TypeError: messageUris[0] is undefined
Source file: chrome://quickfolders/content/quickfolders.js
Line: 1571
 ----------
Mon May 09 2016 09:08:12
Error: TypeError: transferData.first is null
Source file: chrome://global/content/nsDragAndDrop.js
Line: 479

I added one more line during onDragEnter and found that 

dragSession.isDataFlavorSupported("text/x-moz-message") returns false.

I would assume that the drag handler for the message pane uses this flavor for dragging to the folder tree  / desktop as well - do I don't understand why it doesn't work anymore? Any pointers on how to fix it?

Not sure if I can / should / may set the Status to REOPENED.
quickFolders Prerelease for testing.

TEST CASE
1) install
2) pull any folder to the QuickFolders Bar (this will create a toolbar button representing the folder)
3) pull any email from the thread pane onto the button
4) for additional console logs open Addon Options, advanced tab. activate Debug mode. Rightclick debug mode and also activate
extensions.quickfolders.debug.dnd 

EXPECTED BEHAVIOR
1) ondragenter: the "new subfolder" popup dialog is shown
2) ondrop: the email is moved to the folder represented by the QF button

OBSERVED BEHAVIOR
The errors from previous comments are shown. Specifically, the drag flavor "text/x-moz-message" is not supported anymore.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(In reply to Axel Grude [:realRaven] from comment #12)
> Created attachment 8750230 [details]
> QuickFolders prerelease for testing
You want us to debug your add-on? ;-(

If I understand the problem correctly, you have a problem in buttonDragObserver.onDrop at quickfolders.js:1506?

You're saying that "text/x-moz-message" is not supported any more. We still seem to be using it successfully when dragging messages to folders, etc.

I'm not sure, but maybe you're a victim of bug 860857. Take a look at bug 1269713. Read bug 1269713 comment #9. Inspect attachment 8748703 [details] [diff] [review]. That shows you how I fixed dragging address book entries. However, maybe your case is different since you have dragSession as third parameter and we have the data transfer object there. Forgive me if this is the wrong track.
(In reply to Jorg K (PTO during summer, NI me) from comment #13)
> (In reply to Axel Grude [:realRaven] from comment #12)
> > Created attachment 8750230 [details]
> > QuickFolders prerelease for testing
> You want us to debug your add-on? ;-(

I have 2 problems - 
a) firefox remote debugger doesn't connect to the Tb daily for some reasons
b) the actually interesting code is in cpp, which I cannot debug from here

if nsITransferable isn't usable like in the previous versions I would expect a lot more Addons to break; but I may have used it wrong from the first time I started to work on the Addon back in 2009, so hopefully it will be a "one fits all versions" fix.

> 
> If I understand the problem correctly, you have a problem in
> buttonDragObserver.onDrop at quickfolders.js:1506?
> 
> You're saying that "text/x-moz-message" is not supported any more. We still
> seem to be using it successfully when dragging messages to folders, etc.
> 
> I'm not sure, but maybe you're a victim of bug 860857. Take a look at bug
> 1269713. Read bug 1269713 comment #9. Inspect attachment 8748703 [details] [diff] [review]
> [diff] [review]. That shows you how I fixed dragging address book entries.
> However, maybe your case is different since you have dragSession as third
> parameter and we have the data transfer object there. Forgive me if this is
> the wrong track.

OK I will do that - thanks for the very specific pointer. I will have to be a little careful here as I must make sure to stay backwards compatible so I can still run the addon in Postbox / SeaMonkey. But I am happy if I can fix it initially to work in both Daily and Release Tb.
We have a lot of add-on breakage in TB 45, but you reported this against TB 48.

I don't claim to understand bug 1269713 comment #9 fully (quote):
===
The dragstart event is using DataTransfer which now encodes non-recognized string types into a single type and ignores non-string data. This means that non-string data isn't encoded into an nsITransferable, but is kept in the DataTransfer.
===
So there appear to have been changes related to nsITransferable, however, I'm not aware of anything we did to maintain message dragging working after bug 860857.
I can only debug this in Release and possibly the beta, I am not sure whether this is helpful for fixing it in daily. Some questions on changed behavior for dnd of mails:

onDragEnter: function btnObs_onDragEnter(evt, dragSession)

did these parameters change? Should I get the flavor from elsewhere than the dragSession.
Is there a way to enumerate all supported flavors so I can read them in the log? 

I am using dragSession.isDataFlavorSupported("text/x-moz-message") - which returns false

onDrop: function btnObs_onDrop(evt,dropData,dragSession) 

again, [how] did these parameters change? I am looking at dropData.flavour.contentType.
(In reply to Axel Grude [:realRaven] from comment #16)
> I can only debug this in Release and possibly the beta, I am not sure
> whether this is helpful for fixing it in daily. Some questions on changed
> behavior for dnd of mails:
> 
> onDragEnter: function btnObs_onDragEnter(evt, dragSession)
> 
> did these parameters change? Should I get the flavor from elsewhere than the
> dragSession.
> Is there a way to enumerate all supported flavors so I can read them in the
> log? 
> 
> I am using dragSession.isDataFlavorSupported("text/x-moz-message") - which
> returns false
My theory is that dragSession was "nerfed" and I need to look at 

event.dataTransfer.mozTypesAt(0)

instead. Leave it with me, I am rewriting based on 
http://mxr.mozilla.org/comm-central/source/mail/base/content/folderPane.js#677
now as at least that's know to be working.
(In reply to Axel Grude [:realRaven] from comment #16)
I can't answer any of that. I was lucky that Neil Deakin helped me with bug 1269713. I can only suggest that you study what TB does:
https://dxr.mozilla.org/comm-central/search?q=text%2Fx-moz-message&redirect=false&case=false

I have the feeling there are more than one drag and drop scheme. The between-address-book drag apparently implements nsITreeView::CanDrop/onDrop. 

In general, I found it useful to develop add-ons using a local build.

(some of this comment might already be superseded with your comment #17 which crossed wires as I was typing.)
I have written some workaround code which I am happy to report also works in the  (rather outdated) mozilla version of Postbox 4.0

Basically I am not using the dragSession.getData during ondrop anymore for getting the message URIs and use event.dataTransfer.mozGetDataAt(..) instead.

Notes: 
1) dragSession still works for other drag objects such as message folders.
2) dropData.flavour.contentType is set to "text/x-moz-message"

From now, I am using pretty much the code in:
  http://mxr.mozilla.org/comm-central/source/mail/base/content/folderPane.js#823

Patched QuickFolders version in my own bug:
  https://www.mozdev.org/bugs/show_bug.cgi?id=26224
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Resolution: FIXED → WORKSFORME
Glad we could assist ;-)
Resolution: WORKSFORME → INVALID
(In reply to Axel Grude [:realRaven] from comment #19)
> I have written some workaround code which I am happy to report also works in
> the  (rather outdated) Mozilla version of Postbox 4.0

It seems the problem is back I have had 2 users reporting they cannot drop messages onto the buttons anymore in Thunderbird Daily 51.0a1. I can reproduce the bug but do not get an error in console. Dragging of folders is also broken. 

Should I reopen this bug and change the version number or create a new one?
Flags: needinfo?(jorgk)
To give more information - the dragenter / dragover / dragexit events all happen but the drop event is completely omitted.
To me this is an add-on problem and we can't debug it for you. As far as I know, the failure only shows in your add-on and TB is fully working.

You need to pinpoint the problem and if you think it's a C-C or M-C problem, file a new bug appropriately with the exact detail of what you think is wrong.

Also note bug 1299768 comment #1.
Flags: needinfo?(jorgk)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: