104729 - Drag and Drop not implemented under BeOS

Status: RESOLVED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect, P3)

Description

[Paul]

Drag and Drop was not implemented in the BeOS port.

Comment 1

[Paul]

I have written some code to implement Drag and Drop.  Currently, this code does
not support native dnd by default, but most of the code to do so, is there, but
turned off.  BeOS uses BMessages to do dnd, and sends multiple messages between
the target and the source when something gets dropped.  The final message was
not getting through, and since I was unable to find a reason for this, I decided
to at least make the drag and drop work for just mozilla, and worry about native
support later.

Below is the zip file containing the patch, as well as some new files.

Comment 2

[Paul]

Attachment: Patch for DND on BeOS. This does not support native drags.

Comment 3

[Zach Lipton [:zach]]

Nothing to spec, take thee to xpapps: D&D!

Comment 4

[timeless]

it's a good start

Comment 5

[Paul]

DND for bookmarks from the URL bar, mostly works.  DND for mail does not work. 
It calls the GetData, and does get it, and puts it in the transerable object,
but the mail does not get moved.

Comment 6

[Terri Preston]

qa contact -> pmac

Comment 7

[Paul]

updated patch (finally) to come

Comment 8

[Paul]

Attachment: updated patch (native dnd not implemented yet) needs work

I am posting this patch because I need another set of eyes to look at it.  I
cannot, for the life of me, see why this is not working.  If, I remove my
preference directory, the first drag to the toolbar works for me, but no other
drags will.  The "IsDataFlavorSupported" method seems to be working just fine
(in mail, folders become hightlighted on drag over events)  But, GetData is not
always called (in the mail tree) and, even if it is, and gets the data, nothing
is actually dragged.  

I think I've looked at this code too long, and need some help.	It is more than
likely, something stupid I have overlooked.

To see the log, set NSPR_LOG_MODULES=nsDragService:5

Comment 9

[Paul]

Attachment: nsIDragSessionBeOS.h - needed for above patch

Extra metods needed for BeOS implementation

Comment 10

[Paul]

Setting the priority, so I can better organize the issues in bugzilla I need to
take care of

Comment 11

[tqh]

Trying to understand how to do this. I've added nsIDragSessionBeOS.h and the
makefiles (in case it's needed). I'm a bit unsure about nsDragService.cpp
(InvokeDragSession) in the patch and how to do that. If anyone has any pointers
please let me know. 

Comment 12

[tqh]

Attachment: Half-updated old patch

This patch compiles together with nsIDragSessionBeOs.h (in widget/public) but
is not in any way functional. I don't think the changes in nsWindow.cpp (none
in this patch) will be all that hard if I just can figure out nsDragService.cpp
properly. 

(I suspect the old patch would have worked a bit better nowadays since we've
fixed a couple of things since then.)

Comment 13

[tqh]

Attachment: nsDragService updated (drag working)

Ok, think I've translated the old version of DragService. Not sure exactly how
everything is supposed to work, but at least it works for dragging. Only
nsWindow patches left, which I'm a bit more confident about.

Comment 14

[tqh]

Simon, as I've updated DragService.cpp you maybe could take a look and see if
it's  ok. It seems to initiate every drag I've tried so that part might be ok.

Comment 15

[tqh]

Ok, managed to get some drops going. I can now drag links within the same window
and drop them on the address-bar. Some times the page doesn't load properly, but
as I don't do a lot of cleanup right now that is not my main concern. Dragging
between Firefox-windows still seems broken though (so no Customize) or dragging
link to another url-bar.

Comment 16

[Simon]

Good work.

It's going to be another busy week for me though, so if you're happy that what
you have so far works OK, I'll leave it until you've got the drops going before
I take a look.

If you run into dificulties let me know and I'll see if I can find some time to
look at it.

Comment 17

[tqh]

Attachment: Slightly updated nsIDragSessionBeOS.h

Comment 18

[tqh]

Attachment: Working Internal DND, so not from/to other apps yet.

This one seems to work nicely, and if I didn't miss anything in the patch I
suggest we review and commit this work in progress (it's dragging to/from
native apps that needs work). It will fix one bug and users can finally use the
customize feature...

Comment 19

[tqh]

Image dragging depends on bug 294234.

Comment 20

[tqh]

Hmm, seems I missed something in nsWindow from my experimental version, or I
fixed something in it, as customize in Firefox seems to work poorly and maybe
other things as well.

Comment 21

[tqh]

Well it seems it's ok.

Comment 22

[tqh]

Comment on attachment 185337 [details]
Slightly updated nsIDragSessionBeOS.h

r?

Comment 23

[tqh]

Comment on attachment 185337 [details]
Slightly updated nsIDragSessionBeOS.h

Soon replaced (hopefully)

Comment 24

[tqh]

Attachment: How DND nsWindow should be

This is just so I can use the nice diff-function, but I think this is probably
how nsWindow will end up.

Comment 25

[Doug Shelton]

which of the above patches must be applied to test?  all?  just the last? 
Please advise.

Comment 26

[tqh]

With our recent nsWindow changes I'll have to update this, otherwise I think it
will be very complicated to patch if you don't know how it works.

Comment 27

[Doug Shelton]

I can't claim to know how it works, but was willing to see if I could revise the
patch, out of respect for your schedule, tqh.  I'll gladly wait for something to
test if you have time to devote to this project.  I truly appreciate all the
progress you've made in this area.

Comment 28

[tqh]

Ok, I think it's time to close this one, or Niels will get angry with me.

Comment 29

[tqh]

Attachment: Functionally complete DND-handling in nsWindow

This is only nsWindow changes, I will fix the other files as well.

Comment 30

[Sergei Dolgov]

nsWindow part looks ok

Comment 31

[tqh]

Comment on attachment 199590 [details] [diff] [review]
Functionally complete DND-handling in nsWindow

Out with the old...

Comment 32

[tqh]

Attachment: New dragsession interface for BeOS (widget/public)

And in with the new. (I think this one is final.)

Comment 33

[tqh]

Attachment: Makefile.in adds Interface (widget/public)

Also final I think.

Comment 34

[tqh]

Attachment: DND changes in widget/src/beos

I think all but nsDragService.cpp is final. I suggest we check this in as it is
better than current behaviour and the widget/public changes are hardest to
commit. I will then work on improving nsDragService.cpp for DND to and from
other apps...

Comment 35

[tqh]

Comment on attachment 199668 [details] [diff] [review]
DND changes in widget/src/beos

Note that nsDragService.cpp is not complete, but it's a start, and I'd like to
have this first version in CVS and improve it from there. Also removed some
weird line in Makefile.in.

Comment 36

[Sergei Dolgov]

Comment on attachment 199665 [details]
New dragsession interface for BeOS (widget/public)

r=sergei_d

Comment 37

[Sergei Dolgov]

Comment on attachment 199666 [details] [diff] [review]
Makefile.in adds Interface (widget/public)

r=sergei_d

probably needs sr, but unsure if rules weren't changed. Local BeOS-only change
in common file

Comment 38

[Sergei Dolgov]

tqh, which version of widget/src/beos you used for last patch?
I'm getting compilation errors:
    -DMOZILLA_CLIENT -include ../../../mozilla-config.h
-Wp,-MD,.deps/nsDragService.pp nsDragService.cpp
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:57:
warning: multiline `//' comment
In file included from
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:72:
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/dist/include/layout/nsPresContext.h:53:
nsILanguageAtomService.h: No such file or directory
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:74:
nsIView.h: No such file or directory
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:244:
warning: `#ifdef' argument starts with a digit
In file included from
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.h:44,
                 from
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:42:
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/xpwidgets/nsBaseWidget.h:100:
warning: `nsBaseWidget::SetCursor(imgIContainer *, unsigned int, unsigned int)'
was hidden
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsWindow.h:148:
warning:   by `nsWindow::SetCursor(nsCursor)'
In file included from
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:72:
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/dist/include/layout/nsPresContext.h:673:
`nsILanguageAtomService' was not declared in this scope
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/dist/include/layout/nsPresContext.h:673:
template argument 1 is invalid
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/dist/include/layout/nsPresContext.h:673:
ANSI C++ forbids declaration `mLangService' with no type
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:
In method `nsresult nsDragService::InvokeDragSession(nsIDOMNode *,
nsISupportsArray *, nsIScriptableRegion *, unsigned int)':
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:193:
invalid use of undefined type `class nsIView'
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/dist/include/gfx/nsIDeviceContext.h:48:
forward declaration of `class nsIView'
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:204:
invalid use of undefined type `class nsIView'
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/dist/include/gfx/nsIDeviceContext.h:48:
forward declaration of `class nsIView'
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:
In function `static const char * nsDragService::FlavorToBeMime(const char *)':
/mozbuild/home/mozbone/2005-10-08-05-trunk/mozilla/widget/src/beos/nsDragService.cpp:782:
warning: control reaches end of non-void function
`nsDragService::FlavorToBeMime(const char *)' 

Comment 39

[Sergei Dolgov]

it compiles here if i change in 
nsPresContext.h
line
#include "nsILanguageAtomService.h"
to
#include "locale/nsILanguageAtomService.h"
and in nsDragService.cpp line
#include "nsIView.h"
to
#include "view/nsIView.h"
so, something is wrong in configuration

Comment 40

[Sergei Dolgov]

tried to move messages in SeaMonkeyMail from one folder to another.
Mail stays at old place, but some debug info appears in Terminal, like that:
BMessage: what = DATA (0x44415441, or 1145132097)
    entry  be:originator, type='CSTR', c=1, size=8, data[0]: "BeZilla"
    entry   be:clip_name, type='CSTR', c=1, size=18, data[0]: "BeZilla Drag Item"
    entry     be:actions, type='LONG', c=2, size= 4, data[0]: 0x44444350
(1145324368, 'DDCP')
                                                     data[1]: 0x44444d56
(1145326934, 'DDMV')
    entry       be:types, type='CSTR', c=2, size=19, data[0]: "text/x-moz-message"
                                            size=25, data[1]:
"application/octet-stream"
    entry   be:filetypes, type='CSTR', c=1, size=19, data[0]: "text/x-moz-message"
    entry be:type_descriptions, type='CSTR', c=1, size=19, data[0]:
"text/x-moz-message"
mailCharsetLoadListener: windows-1251                                          
        

Comment 41

[tqh]

And that works on other platforms?

Comment 42

[Sergei Dolgov]

Sure it works. This i way i'm sorting my mails under MS Windows Mozilla.
Though, bookmark dragging in SeaMonkey Bookmark manager also does nothing
besides those terminal messages.
Sure, when i try to drag something, no dragging icon or any other sign
appearing, but destination gets marked/selected for moment when i try to drop there.
But no any other effect.

So give me what is supposed to work at this stage and how to test it.

Comment 43

[Sergei Dolgov]

Additional notices. 
1)Draggin bokmarks works, but only half of that. It appears on new place, but
remains at old
2)I know that outside dragging shouldn't work yet, but i tried - dragged image
file from desktop to Mozilla. It resulted in downloading not image, which i
dragged, but last dragged bookmark.
Maybe it helps

Comment 44

[Sergei Dolgov]

Also found crash bug.
With following lines
	Window()->CurrentMessage()->FindInt32("buttons", (int32 *)&buttons);
	if(NULL == msg && !restoreMouseMask && buttons)
		return;
in nsViewBeOS::MouseMoved
application crashes at mDragMessage->FindString
in nsDragService::IsInternalDrag()
if you try to drop something from outside onto (freshly launched) app

Comment 45

[Sergei Dolgov]

Comment on attachment 199668 [details] [diff] [review]
DND changes in widget/src/beos

1)in
IsInternalDrag you don't test if mDragMessage != 0
2)in UpdateDragMessageIfNeeded you neither check mDragMessage !=0 neither 
assign aDragMessage to it.

This is reason of crash from previous notice.
And that was serious crahs - if user even don't try to drag anything at
Mozilla, but just moves some item over desktop for other purpose, and crosses
mozilla border occasionally - Mozilla wil crash. 
Unacceptable

Comment 46

[Sergei Dolgov]

tqh, please look also at comment
https://bugzilla.mozilla.org/show_bug.cgi?id=298171#c11
Maybe I'm missing some logic there?

Comment 47

[Sergei Dolgov]

btw, totally after removed statement
if(NULL == msg && !restoreMouseMask && buttons) return;
in MouseDown(), i got DnD to work at least partially - now i can select links
and drop those on tabs, so URL opens there

Comment 48

[Sergei Dolgov]

sorry, links DnD work also without change from previous comment.
So, at least something is working, but group
of{UpdateDragMessageIfNeeded;IsInternalDrag;MouseMoved} must be fixed anyway,
even with that preliminary stage.
Actually i dislike nested "break;s" in MouseMoved, looks like ancient BASIC
program with mad GOTO placement, but i'm planning to change all mouse-related
code anyway in future, after we sort DnD things out.

Comment 49

[Niels Reedijk]

I'm very happy.

Comment 50

[tqh]

*personal opinion ahead*
Ew, don't say that about breaks. They are very well defined compared to goto's
and their state is very easy to follow. When you call a break  it's very simple
to see that this is an exitpoint out of the block, you can't see that with
if/elses which means you have to keep the whole block in your head in order to
see when and how it exits. You know that the code will continue after the block
as well. 

Break and continue are nice and powerful, while keeping code very readable.
Compared to long if/else-blocks that almost always gets messy, nested and 'goto'
like. Most times you even have to rely on formatting to even be able  to read
if/else-statements. The if/else code isn't always the answer even though most
ppl seem to think so. 

Comment 51

[tqh]

> 2)in UpdateDragMessageIfNeeded you neither check mDragMessage !=0 neither 
> assign aDragMessage to it.

Yes that is a part that should be 'not yet implemented'. It's only done on
native drags, and this version isn't complete in that regard.

Comment 52

[Sergei Dolgov]

Comment on attachment 199668 [details] [diff] [review]
DND changes in widget/src/beos

Ooops, it seems i mistakenly granted review.
Two things must be fixed before commit:
1)mentioned crash in UpdateMessafeIfNeeded - maybe by adding input parameter to
IsInternal and feeding it with aDragMessage/mDragMessage depending on context
2)Strange compilation problems - maybe it is my propblem and i need just fresh
tree and patched mozilla/widget before global configure/make? At least i wait
explanations or ideas on that

Comment 53

[Sergei Dolgov]

problem 2) is clear now.
mozilla/widget/src/beos/Makefile.in
needs adding to 
REQUIRES	= xpcom \ section:
 view \
 locale \
and maybe removing timer ? (as you remove timer  in other place in Makefile.in)

Comment 54

[Christian :Biesinger (don't email me, ping me on IRC)]

widget/public files checked in

Checking in Makefile.in;
/cvsroot/mozilla/widget/public/Makefile.in,v  <--  Makefile.in
new revision: 1.101; previous revision: 1.100
done
RCS file: /cvsroot/mozilla/widget/public/nsIDragSessionBeOS.h,v
done
Checking in nsIDragSessionBeOS.h;
/cvsroot/mozilla/widget/public/nsIDragSessionBeOS.h,v  <--  nsIDragSessionBeOS.h
initial revision: 1.1
done

Comment 55

[tqh]

Attachment: DND changes in widget/src/beos 2

Corrected IsInternalDrag (made it a static function that takes a BMessage) and
added missing Makefile.in changes. I must've been tired yesterday.

Comment 56

[Sergei Dolgov]

Comment on attachment 199734 [details] [diff] [review]
DND changes in widget/src/beos 2

r=sergei_d
functionality is minimalistic at the moment, but let it stay in tree - it eases
development

Comment 57

[Sergei Dolgov]

https://bugzilla.mozilla.org/attachment.cgi?id=199734
landed in trunk
Checking in mozilla/widget/src/beos/Makefile.in;
/cvsroot/mozilla/widget/src/beos/Makefile.in,v  <--  Makefile.in
new revision: 1.48; previous revision: 1.47
done
Checking in mozilla/widget/src/beos/nsDragService.cpp;
/cvsroot/mozilla/widget/src/beos/nsDragService.cpp,v  <--  nsDragService.cpp
new revision: 1.14; previous revision: 1.13
done
Checking in mozilla/widget/src/beos/nsDragService.h;
/cvsroot/mozilla/widget/src/beos/nsDragService.h,v  <--  nsDragService.h
new revision: 1.5; previous revision: 1.4
done
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.99; previous revision: 1.98
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.38; previous revision: 1.37
done 

Comment 58

[tqh]

Closing bug. Finally. All changes to DND will be done in new bugs.
nsDragService.cpp needs more polishing.

Comment 59

[Niels Reedijk]

Attachment: Same patch for the 1.8 branch

This is a BeOS-only change. This is a backport of attachment 199734 [details] [diff] [review] . It will not affect the other platforms in any way.

Comment 60

[Niels Reedijk]

Comment on attachment 199666 [details] [diff] [review]
Makefile.in adds Interface (widget/public)

Requesting approval for the 1.8 branch. This is a BeOS only change in shared territory, but it has been in the MAIN branch for a while and hasn't caused any problems. Impact of other platforms is non-existant.

Comment 62

[Niels Reedijk]

Comment on attachment 199665 [details]
New dragsession interface for BeOS (widget/public)

Accidentaly touched the review. View bug history to confirm that sergei_d reviewed this file.

Comment 63

[Niels Reedijk]

Checked in by timeless.

Comment 64

[Sergei Dolgov]

Tqh, I'm bit confused by content of nsIDragSessionBeOS.h

There are two methods declared:
    NS_IMETHOD SetDragMessage(BMessage *aDragMessage) = 0;
    NS_IMETHOD GetDoingDrag(PRBool *aDoingDrag) = 0;

But i don't see any sign of implementation, even let it be stubs.

Comment 65

[tqh]

(In reply to comment #64)
> Tqh, I'm bit confused by content of nsIDragSessionBeOS.h
> 
> There are two methods declared:
>     NS_IMETHOD SetDragMessage(BMessage *aDragMessage) = 0;
>     NS_IMETHOD GetDoingDrag(PRBool *aDoingDrag) = 0;
> 
> But i don't see any sign of implementation, even let it be stubs.
> 

Are you sure?
http://lxr.mozilla.org/seamonkey/source/widget/public/nsIDragSessionBeOS.h

Comment 66

[Sergei Dolgov]

Comment 65:
There are only declaration there, in your link, as I mentioned already.
LXR cannot find any occurance besides that file.
For other platforms methods from nsIDragSessionPLATFORM.h are implemented in nsDragService files.

Comment 67

[Sergei Dolgov]

or do you mean

 = 0;

as stubs? (with meaning notimplemented)

Comment 68

[tqh]

They arn't declared at all in the lxr I linked. It only inherits nsISupports. So either you mean another header or I'm totally confused.