343190 - DnD month view: Drag and Drop stops working if event is dropped out of calendar view

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Stefan Sitter [:ssitter]]

Fall out from Bug 312736:
DnD month view: Drag and Drop stops working if event is dropped out of calendar view 

Steps to Reproduce:
1. Switch to month view
2. Create normal event
3. Drag and Drop that event out of calendar view, e.g. to task list
   --> Drop shadow is still displayed at the last displayed position.
4. Refresh view (e.g. select 'Go to Today').
   --> Event is shown at original position.
5. Drag and Drop that event to another day.

Actual Results:
Drag and Drop is not possible at all. JavaScript console shows:
    Error: shadow.parentNode has no properties
    Source File: chrome://calendar/content/calendar-month-view.xml 
    Line: 471

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060629 Sunbird/0.3a2+ (my own trunk build)

Comment 1

[Joey Minta]

So the obvious solution here is for the dropping code to also add a listener to its containing window for an ondrop event.  This solves the bug as described.  The tougher case is where someone drops our data outside the app (either successfully or unsuccessfully).  As far as I know, we have no way of getting this information at all.  I'm looking for opinions on a.) Is this simply an edge case we don't care about?  b.) Is it sufficient to simply update ourselves the next time we're moused over?  In this case, the same fix for dropping outside the app would work for dropping inside the app. or c.) Is this something we need to lobby heavily for gecko to care about?

Comment 2

[Reed Loden [:reed]]

The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!

Comment 3

[Joey Minta]

Attachment: add mouseover listener

Gosh, it's painful to have to write code this ugly.  Such is the price of playing with mozilla dnd code...

Patch adds a mouseover listener, to check whether we're really still dragging.  If we dropped somewhere else, we'll revert the drag gracefully.

Comment 4

[Joey Minta]

We need to add a listener for when we leave the view, and then register this kind of listener.  We should also put the event back and kill the shadow at this point.  If that's a lot of work, we'll land this.

Comment 5

[Matthew (lilmatt) Willis]

Comment on attachment 231046 [details] [diff] [review]
add mouseover listener

Moving reviews to ctalbert and lilmatt per dmose

Comment 6

[cmtalbert]

Comment on attachment 231046 [details] [diff] [review]
add mouseover listener

This looks great. I tested this on WinXP. The only nit I have is that the window.addEventListener and the two window.removeEventListener lines could be wrapped and they would be under 80 chars. r+

Comment 7

[Matthew (lilmatt) Willis]

Comment on attachment 231046 [details] [diff] [review]
add mouseover listener

>Index: calendar/base/content/calendar-month-view.xml
>+        monthbox.monthView.dropListener = function checkStillDragging() {
>+            var dragService = Components.classes["@mozilla.org/widget/dragservice;1"]
>+                                    .getService(Components.interfaces.nsIDragService);
Align this line ^^^

>+             var session;
>+             try {
>+                 session = dragService.getCurrentSession();
>+             } catch(ex) {}
>+             if (!session) {
>+                 monthbox.removeDropShadows();
>+                 monthbox.addItem(item);
>+                 window.removeEventListener("mouseover", monthbox.monthView.dropListener, true);
>+             }
Somehow all of this got indented one space too much.
Also, please add a space between "catch" and the (, and wrap the window.removeEventListener line so it's under 80 chars.

>+        };
>+        window.addEventListener("mouseover", monthbox.monthView.dropListener, true);
Wrap this line also.

>+         window.removeEventListener("mouseover", this.monthView.dropListener, true);
Wrap this line also, and what's up with the wacky indenting in this area?

r+ with those fixed.

Comment 8

[Joey Minta]

Comment on attachment 231046 [details] [diff] [review]
add mouseover listener

need a peer here.

Comment 9

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 231046 [details] [diff] [review]
add mouseover listener

r2=mvl

Comment 10

[Joey Minta]

Patch checked in.