343736 - [BeOS] Finish implementation of native Drag And Drop

Status: RESOLVED WONTFIX

(Core Graveyard :: Widget: BeOS, defect)

Description

[Sergei Dolgov]

Place to collect all wisdom about native DnD.

Comment 1

[tqh]

I know how to make it work, but don't have the time or energy to finish it.

The code is mostly there in it's current form. Only really lacking thing to do is convert between BeOS-mime and Mozilla's and decide which are useful to native DND and which arn't.

The key functions to the approach:
nsDragService::StartDragSession and nsDragService::EndDragSession are called when mouse enters/exits view with or without drag-message. So we can end DND-sessions that ends outside, and start sessions that start outside of Mozilla. These are already in place (although they might be moved inside UpdateDragMessageIfNeeded and only done on certain checks).

nsDragService::UpdateDragMessageIfNeeded is called om mouse enter / exit view to make sure that the data we're dragging is the correct one. Right now it only saves the most recent dragmessage.

nsDragService::TransmitData()
When a negotiation message that asks for data (and action) arrives in nsViewBeOS::MessageReceived TransmitData() is called to send the data to the requesting app finishing of a DND to another application. This needs to be implemented to send a correct respone.

nsDragService::CreateDragMessage()
This creates a dragmessage for drags that starts inside Mozilla, and needs proper mappings of mime-types to describe what data Mozilla can offer to other apps. Other than that it's probably ok.

So, if TransmitData and CreateDragMessage are implemented it should really just spring to life.

This is from the top of my head, but I gave this a lot of thought and testing when I was working on the first version. The big problem for BeOS is that we don't have any way of listening to drags directly. Adding URL to DND-topic in Bebook as well.

Comment 2

[Sergei Dolgov]

Attachment: patch to reduce "noise

http://lxr.mozilla.org/seamonkey/source/widget/public/nsIDragService.idl#73

Reducing activity in MouseMove and ONDROP - to call dragservice methods only in case of external drag

Not for checking but for discussion

Comment 3

[tqh]

Yes, something along those lines should probably work.

Comment 4

[tqh]

Although the EndDragSession in CallMethod 'drops' should probably be done on all drops, not just remote ones.

Comment 5

[Sergei Dolgov]

Per comment #4:

Does it? If I call EndDragSession in drop before widget release, internal drag is half-broken. Like it was with mail and bookmarks.

If I move it past widget release, as it is now, all works, but debug printf/output in in our EndDragSession isn't called, at least with internal drops for mailnews and bookmarks.

That is.

So proper solution looks like calling it before widget release, but only for external/remote drag.

Comment 6

[tqh]

Not sure if I missed another place where you call EndDragSession for internal drags, the position is ok, but should the 
  if (dragService && (bool)info->args[3])
be like that?

Shouldn't it still be
  if (dragService) 

(If EndDragSession for internal is done somewhere else I guess it's ok.)

Comment 7

[Sergei Dolgov]

according to link I gave in comment to patch (nsIDragService) you call explicitly Start/EndDragSession for external drag, not internal.

And, as I said, internal drop works absolutley perfectly WITHOUT EndDragSession, but works flacky if it is called and reaches its target (it reaches target if placed before widget release).

So this think needs bit more understanding, looks like internal drag don't need any of those methods at all, as it can suppose from nsIDragSession.

I will try to investigate it bit more.

Comment 8

[tqh]

Yes, I think it is only a matter of where we should call it. Or if we can live without it. Havn't looked at that function right now to see if we, or xpwidgets does anything there.

Comment 9

[Sergei Dolgov]

Experimenting with several aspects of external drag, I noticed that patch
https://bugzilla.mozilla.org/attachment.cgi?id=228095
for Bug 343569
has potential logical flaw, inspite now it works nicely.

1)Code to reset action type shouldn't be called at all if there is single and only action set in given mask.
2)If mask is combined from 2 or 3 possible actions, much more complicated logic required to mix it with user choice.

Probably I will submit patch for 1) soon, but 2) is doubtful, as it may result in non-elegant messy code.

Comment 10

[Sergei Dolgov]

One more notice.
It may be helpful for native drag and drop to call SetDragAction(*NONE*) inside our nsDragService::EndDragSession(), as gtk version does.

Comment 11

[tqh]

Sounds like good ideas.

Comment 12

[Sergei Dolgov]

Per comment 11:
Which one - in comment 9 or comment 10?

For idea in comment 10 we need keep original action mask stored somewhere and update actual mask from MouseMoved via UpdateDragInfoIfNeeded permanently. In other case we are in danger to nullify action mask resolutely when crossing BView borders:) So this is really for future and makes our port to behave more like other platforms

Comment 13

[Sergei Dolgov]

Another tip for future:
mDragService = do_GetService(kDragServiceContractID);
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsDragHelperService.cpp#88

Cache service reference if possible - make dragging more lightweight. At least while we are inside same BView.

Comment 14

[Sergei Dolgov]

All tier-1 versions use explicit timer for drag-over - 16 Hz in Win32 and 10 Hz in gtk and mac. I wonder if it is gecko requirement or platform issues, or mouse move performance workaround-improvement
Currently we simply call drag-over from MouseMoved with transit==0 && msg != 0

Comment 15

[Sergei Dolgov]

Next notice.
Other platforms reset (set to false) canDrop at each tracking/DragOver call.
Marking it as very important action and telling that event handlers (in Gecko) should set it then properly by self.

Comment 16

[tqh]

I left canDrop in the work I did as it was not necessary to prove working DND. It should be implemented, but it's not hard to do.

I don't think we should cache dragservice. We waste some mem in every obj where we cache it, and we might prevent the com-model to only instantiate it when it's needed as we always keep a lot of refs. If do_GetService should be lightweight by itself (maybe not first call), unless the Mozilla devs have done a poor job.

I don't think we need a timer, we have a thread for every win, the other platforms have a timer. IIRC.

Comment 17

[Carlos Alén Silva]

Is this a WONTFIX since it's a BeOS bug?

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Nothing has been happening on the beos side for a while, closing.