dragleave is fired on previous drop target after successful drop

RESOLVED FIXED in mozilla11

Status

()

Core
Drag and Drop
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

968 bytes, text/html
Details
1.60 KB, patch
Neil Deakin (not available until Aug 9)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 571179 [details]
test case

Use the provided test case as follows:

1) start dragging the green rectangle
2) drag it over the red rectangle
3) drop it
4) "status" should be "drop" now
5) start dragging the green rectangle again

Actual:

"status" is "drop dragleave" because this was fired again on sLastDragOverFrame (the red rectangle) from the previous dnd operation.

Expected:

"status" should still be "drop".
(Assignee)

Comment 1

6 years ago
Created attachment 571183 [details] [diff] [review]
patch v1

I don't know if that's the right approach but it works for my test case. This patch sets sLastDragOverFrame to null when starting a new drag operation and thus cleaning out the value from previous drag operations.
Attachment #571183 - Flags: review?(karlt)
Enn or smaug would be better reviewers here.
Comment on attachment 571183 [details] [diff] [review]
patch v1

Guessing smaug looked at this most recently.
Attachment #571183 - Flags: review?(karlt) → review?(bugs)
FWIW ISTR this behaviour even before bug 591249 landed.
Would it be better to clear this during the dragend event (in PostHandleEvent)?

Comment 6

6 years ago
Is this a regression? If it is, is it perhaps from bug 591249

Comment 7

6 years ago
Comment on attachment 571183 [details] [diff] [review]
patch v1

Neil, could you review this.
Attachment #571183 - Flags: review?(bugs) → review?(enndeakin)

Comment 8

6 years ago
(If you don't have time to review, I can review)
(In reply to Neil Deakin from comment #5)
> Would it be better to clear this during the dragend event (in
> PostHandleEvent)?

I assume that wouldn't be suitable when the source node is external to the browser.
Does the bug occur for an external drag? Shouldn't the node be cleared in dragexit?

Regardless, there's no harm is clearing sLastDragOverFrame both in dragend and during dragstart. That would make it better in case someone adds some code to check sLastDragOverFrame outside a drag for whatever reason.
(Assignee)

Comment 11

6 years ago
Created attachment 571604 [details] [diff] [review]
patch v2

I guess it would be sufficient to only clean sLastDragOverFrame in PostHandleEvent[NS_DRAGDROP_DROP] (it's already set to null on NS_DRAGDROP_EXIT) but I kept the change from the first patch to also clean out on dragstart like you said.
Attachment #571183 - Attachment is obsolete: true
Attachment #571183 - Flags: review?(enndeakin)
Attachment #571604 - Flags: review?(enndeakin)
(Assignee)

Updated

6 years ago
Duplicate of this bug: 623214
Comment on attachment 571604 [details] [diff] [review]
patch v2

I'd actually meant to put it in 'dragend' rather than 'drop' but this is probably ok.
Attachment #571604 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 14

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/cad93c498146
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 15

6 years ago
https://hg.mozilla.org/mozilla-central/rev/cad93c498146
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.