Closed Bug 541520 Opened 10 years ago Closed 10 years ago
dragleave fired at successful drop
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 GTB6 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 GTB6 (.NET CLR 3.5.30729) This page is a simple drag/drop example page. The drop target with the text starts as display:none and is changed to display:table in the dragenter event. If you drop a file onto the target, you get no drop event. If you drop it elsewhere, you do get a drop event. Reproducible: Always Steps to Reproduce: 1. Drag a file from the desktop onto the page 2. Drop it where it says "Drop 'em here" Actual Results: No drop event fired Expected Results: Drop event
Would be great if you could attach the testcase so that it doesn't get lost in the future.
Component: General → Drag and Drop
Product: Firefox → Core
QA Contact: general → drag-drop
My apologies; the default entry form has space for a URL, but not for an attachment. I'll do that in the future.
Could it be that the "dragleave" event fires before the "drop" event. Thus the drop area gets hidden before the actual drop can take place?
That is indeed what's happening - if I delay the hiding by putting it in a timeout, I do get the drop. I'm not sure what the spec says about that ordering.
It seems like we are firing the events in the correct order per the HTML5 spec. However it also seems like if the element is visible at the time when the user ends the drag operation by releasing the mouse button, then we should still fire the "drop" event at the element. Even if the element is display:none by the time that we try to dispatch the event.
The dragleave event shouldn't actually be firing when a successful drop occurs, only if it failed or was cancelled. I discovered this issue yesterday.
Neil, is this something you're going to fix? Is there a separate bug? I'm putting a workround in our code and a link to this bug so we can remove it eventually.
Confirming and updating summary
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Drop event doesn't fire on made-visible targets → dragleave fired at successful drop
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #423589 - Flags: review?(jonas)
Attachment #423589 - Flags: review?(jonas) → review+
r=me on the non-mac parts. Assuming (and hoping) that's what you wanted josh to review.
Comment on attachment 423589 [details] [diff] [review] don't fire dragleave on drop, on when a drag is cancelled Steven was just looking at this stuff, if he's fine with it I am too.
Attachment #423589 - Flags: review?(joshmoz) → review?(smichaud)
Attachment #423589 - Flags: review?(smichaud) → review+
Comment on attachment 423589 [details] [diff] [review] don't fire dragleave on drop, on when a drag is cancelled I haven't tested this, but it looks fine to me. I assume the change is to ensure that a NS_DRAGDROP_EXIT message is always sent (instead of a NS_DRAGDROP_DROP message) when a drop occurs over a target where the drop is disallowed.
Can we leave out the eventUtils hunk here? We can get the test ironed out in bug 462172 instead?
Sure, I'll check it in without that change.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Is this 3.6.1-worthy?
I think it would be nice to backport given how small the patch is. However I'll leave it up to Neil to judge how safe it is since this type of things tend to be fairly sensitive (though Neil has done a great job on making that less true)
The two parts of the bug: - the firing dragleave on drop behaviour was added when implementing the spec so anything pre-3.5 including current firefox/thunderbird/etc shouldn't have any issues with this change. - the Mac change fixes dragleave to fire when a drag is canceled which was already correctly occurring on Windows and Linux. So I think this should be a very safe change.
Neil, would you be able to come up with a 1.9.2 patch? Or check if the current one applies cleanly?
Dragging leaves a stripe (insertion line) in the menu, which is still present after closing and reopening.
(In reply to comment #21) > Dragging leaves a stripe (insertion line) in the menu, which is still present > after closing and reopening. Bookmarks menu? If so, file a separate bug and cc places folks.
And mark the bug as blocking this one so that we keep track of regressions for when/if we backport.
The patch applies ok and works on 1.9.2.
I had the impression that this ended up having to land with some other changes in order to not turn the tree orange, such as changes to tests? Maybe I'm misremembering? In any case, we need bug 544047 fixed before we can land this on branch, right?
We took out the changes to EventUtils but that function isn't called in the tree so it shouldn't be a problem.
You need to log in before you can comment on or make changes to this bug.