27791 - linux DND doesn't have correct visual drag feedback

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Christopher Blizzard (:blizzard)]

Right now the linux DND code doesn't properly do drag feedback.  I'm opening
this bug to track the issue.

Don't panic, I have a plan.

According to Owen's Brain we need to make sure that we call gdk_drag_status
exactly once per motion event, possibly after doing a get_data due to lame
protocol restrictions.  I can do this by tracking the beginning on a motion
event, letting the layout do what it needs to do behind the shed and then let
the drag service send the drag status message.  I have to change the
nsIDragSessionGTK service to support this...not a big deal.

Comment 1

[Christopher Blizzard (:blizzard)]

I have a fix for this.  I'm waiting for pav to stop watching X files and review it.

I've also got information that I'm going to post that's from email that I sent
to pink about the way that the can drag property is set on the drag service.
Email cut-n-paste follows...

It would be really nice to give drag feedback via the icon as to whether or not
an area was a valid drag target.  The facility is there via the drag service
canDrop attribute:

 // touch the attribute to trigger the repaint with the drop feedback.
      if ( validFlavor ) {
        //XXX this is really slow and likes to refresh N times per second.
        var toolbar = document.getElementById("PersonalToolbar");
        toolbar.setAttribute ( "dd-triggerrepaint", 0 );
        dragSession.canDrop = true;
        event.preventBubble();
      }

If the flavor is valid and then you drag to another widget where the drag flavor
is not valid, that canDrop attribute remains true, even though the flavor is not
valid.  Would it make sense to set the canDrop to false if (!validFlavor)
without setting the preventBubble()?  This would give other objects the chance
to accept the event and set themselves as a valid drag target, right?

I can also set the feedback in the IsDataFlavorSupported method by always
setting mDragStatus to the last flavor tested.  That's not really the way to do
it, though.  It's a hack.

Comment 2

[Christopher Blizzard (:blizzard)]

I've fixed in the unix stuff that needs to be done to fix this problem.  pink,
can you have a look at my comments?

Comment 3

[Christopher Blizzard (:blizzard)]

I'm taking care of this for pink.  Also adding mail/news people just so they are
in the loop.

Comment 4

[Christopher Blizzard (:blizzard)]

pink and I talked about this.  I've discovered that the solution that I've
proposed here in the JS code won't work.

The problem is that there are a lot of times when you will cross from one
nsIWidget to another and the drag service will be left with canDrop = true. 
Since it's possible that there is no JS event fired for the new widget that is
being dragged over, the canDrop attrib will be left as true even though it's not
a valid drag target.

The real fix is to just always assume that you can't drag for each motion event
and if someone updates you so that you can, then you set it to true.

Pink has said that he will do win32 and mac.

Comment 5

[Mike Pinkerton (not reading bugmail)]

is this a problem on win32 as well? it's not a problem on macOS because there is 
no "drop will succeed" feedback in the cursor. Can someone verify win32 is ok for 
me?

Comment 6

[Christopher Blizzard (:blizzard)]

Attachment: patch for linux

Comment 7

[Christopher Blizzard (:blizzard)]

Can someone ( Pavlov? ) review this patch for me?  Thanks.

Comment 8

[Phil Peterson]

pink, if you're asking whether win32 gives appropriate "drop is legal" vs. "drop
is illegal" feedback in the cursor, it does. Assume we're using OLE D&D at the
platform-specific level, OLE handles cursor changes internally, based on the
flavor (FORMATETC) negotiation.

Comment 9

[Christopher Blizzard (:blizzard)]

Ok, it looks like win32 works ( I guess we're just lucky that way ) and mac
doesn't need anything.  Once pav gets out of bed and reviews this, I'll close
it.

Comment 10

[Mike Pinkerton (not reading bugmail)]

right, we are, i was just wondering if anyone had seen it show the "drag will be 
accepted" cursor in places they shouldn't on win32. I haven't tried d&d in mail 
or the browser recently enough to recall if that was working. Thanks for the info 
;)

Comment 11

[Christopher Blizzard (:blizzard)]

Ok, linux fix checked in, yo.

Comment 12

[Eli Goldberg]

pavlov is done with X-Files and is back crushing bugs at Netscape; per chat with
him, rubber-stamping as Verified Fixed without inspection.

(I'd be happy to verify it personally if anyone would like to provide what the
'correct' visual drag feedback is, and what types of scenarios would trigger it.)

Comment 13

[Christopher Blizzard (:blizzard)]

Please ignore the spam.  Changing address.

Comment 14

[Christopher Blizzard (:blizzard)]

bustage from my reassign

Comment 15

[Christopher Blizzard (:blizzard)]

bustage from my reassign

Comment 16

[Christopher Blizzard (:blizzard)]

bustage from my reassign