367110 - Missing Thunderbird integration for copy/paste

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Michael Büttner (no reviews TFN)]

Lightning's copy/paste functionality is currently not integrated into Thunderbird. This bug is dedicated to provide the missing bits and pieces.

Comment 1

[Michael Büttner (no reviews TFN)]

First of all the command handler for the appropriate commands (cut,copy,paste) is missing, this can easily be hooked up into messenger-overlay-sidebar.js which should delegate the calls to clipboard.js where the necessary functions are already present. clipboard.js can be used out-of-the-box, besides the following issues:

1) deleteEventCommand() is used at http://lxr.mozilla.org/seamonkey/source/calendar/resources/content/clipboard.js#100
This function is defined in calendar.js which isn't available in lightning. I'd suggest moving deleteItems() and deleteEventCommand() from calendar.js to calendarUtils.js (or calUtils.js after bug #366560 has been checked in).

2) getDefaultCalendar() is used at http://lxr.mozilla.org/seamonkey/source/calendar/resources/content/clipboard.js#254
this could be written as '"ltnSelectedCalendar" in window ? ltnSelectedCalendar() : getDefaultCalendar();' in order to make this work in lightning as well as in sunbird. another option would be to provide a helper function for this.

3) document.getElementById("view-deck") is used at http://lxr.mozilla.org/seamonkey/source/calendar/resources/content/clipboard.js#255
which doesn't work in lightning. this can be replaced by "currentView()" since this helper function is already available.

Comment 2

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

this patch implements the bits and pieces as described in above comment.

Comment 3

[Matthew (lilmatt) Willis]

Comment on attachment 251627 [details] [diff] [review]
patch v1

>--- mozilla_ref/calendar/lightning/content/messenger-overlay-sidebar.js	2007-01-02 16:08:53.000000000 +0100
>+var CalendarController =
>+{
>+  supportsCommand: function(command)
>+  {
Make unanonymous, perhaps function ccSC(command) { ?
Put { on same line

>+    switch ( command )
>+    {
Remove spaces around command and put { on same line.

>+      case "cmd_cut":
>+      case "cmd_copy":
>+      case "cmd_paste":
>+        return true;
>+    }
>+
Remove that line^^^

>+    return false;
>+  },
>+
>+  isCommandEnabled: function(command)
>+  {
Make unanonymous, perhaps function ccICE(command) { ?
Put { on same line

>+    switch ( command )
>+    {
Remove spaces around command and put { on same line.

>+      case "cmd_cut":
>+      case "cmd_copy":
>+        return currentView().getSelectedItems({}).length != 0;
>+      case "cmd_paste":
>+        return canPaste();
>+    }
>+
Remove that line^^^

>+    return false;
>+  },
>+
>+  doCommand: function(command)
>+  {
Make unanonymous, perhaps function ccDC(command) { ?
Put { on same line

>+    // if the user invoked a key short cut then it is possible that we got here for a command which is
>+    // really disabled. kick out if the command should be disabled.
Wrap this comment less than 80 chars

>+    if (!this.isCommandEnabled(command)) return;
use { } and put the return on the next line.

>+
>+    switch ( command )
>+    {
Remove spaces around command and put { on same line.

>+      case "cmd_cut":
>+        cutToClipboard();
>+        break;
>+      case "cmd_copy":
>+        copyToClipboard();
>+        break;
>+      case "cmd_paste":
>+        pasteFromClipboard();
>+        break;
>+    }
>+  },
>+
>+  onEvent: function(event)
>+  {
Make unanonymous, perhaps function ccOE(command) { ?
Put { on same line.
Add a // no-op comment here.


>+++ mozilla/calendar/lightning/content/messenger-overlay-sidebar.xul	2007-01-16 13:24:03.093750000 +0100
>+++ mozilla/calendar/lightning/jar.mn	2007-01-16 13:24:32.937500000 +0100
>@@ -42,16 +42,17 @@ calendar.jar:
>   content/calendar/datetimepickers/datetimepickers.xml (/calendar/resources/content/datetimepickers/datetimepickers.xml)
>   content/calendar/datetimepickers/minimonth.css (/calendar/resources/content/datetimepickers/minimonth.css)
>   content/calendar/datetimepickers/minimonth.xml (/calendar/resources/content/datetimepickers/minimonth.xml)
>   content/calendar/printDialog.js                (/calendar/resources/content/printDialog.js)
>   content/calendar/printDialog.xul               (/calendar/resources/content/printDialog.xul)
>   content/calendar/publish.js                   (/calendar/resources/content/publish.js)
>   content/calendar/publishDialog.js             (/calendar/resources/content/publishDialog.js)    
>   content/calendar/publishDialog.xul            (/calendar/resources/content/publishDialog.xul)
>+  content/calendar/clipboard.js (/calendar/resources/content/clipboard.js)
Align the ( beneath the publishDialog one. It should be like 49 chars in.


>+++ mozilla/calendar/resources/content/calendar.js	2007-01-16 13:20:18.171875000 +0100
>@@ -259,48 +259,16 @@ function getSelectedCalendarOrNull()
>    var selectedCalendarItem = document.getElementById( "list-calendars-listbox" ).selectedItem;
>    
>    if ( selectedCalendarItem )
>      return selectedCalendarItem.calendar;
>    else
>      return null;
> }
> 
>-/**
>-*  This is called from the unifinder's delete command
>-*
>-*/
>-function deleteItems( SelectedItems, DoNotConfirm )
>-{
>-    if (!SelectedItems)
>-        return;
>-
>-    startBatchTransaction();
>-    for (i in SelectedItems) {
>-        var aOccurrence = SelectedItems[i];
>-        if (aOccurrence.parentItem != aOccurrence) {
>-            var event = aOccurrence.parentItem.clone();
>-            event.recurrenceInfo.removeOccurrenceAt(aOccurrence.recurrenceId);
>-            doTransaction('modify', event, event.calendar, aOccurrence.parentItem, null);
>-        } else {
>-            doTransaction('delete', aOccurrence, aOccurrence.calendar, null, null);
>-        }
>-    }
>-    endBatchTransaction();
>-}
>-
>-
>-/**
>-*  Delete the current selected items with focus from the unifinder list
>-*/
>-function deleteEventCommand( DoNotConfirm )
>-{
>-    var SelectedItems = currentView().getSelectedItems({});
>-    deleteItems( SelectedItems, DoNotConfirm );
>-}
> 
> 
> /**
> *  Delete the current selected item with focus from the ToDo unifinder list
> */
> function deleteToDoCommand( DoNotConfirm )
> {
>    var SelectedItems = new Array();
>diff -a -x CVS -U 8 -pN -rN mozilla_ref/calendar/resources/content/calendarUtils.js mozilla/calendar/resources/content/calendarUtils.js
>--- mozilla_ref/calendar/resources/content/calendarUtils.js	2006-10-30 22:10:47.000000000 +0100
>+++ mozilla/calendar/resources/content/calendarUtils.js	2007-01-16 13:20:18.187500000 +0100
>@@ -117,16 +117,48 @@ function now()
> 
> function jsDateToDateTime(date)
> {
>     var newDate = createDateTime();
>     newDate.jsDate = date;
>     return newDate;
> }
> 
>+/**
>+*  Delete the current selected items with focus from the unifinder list
>+*/
Indent the bottom 2 comment lines one space for proper javadoc style.

>+function deleteEventCommand( DoNotConfirm )
Remove spaces around DoNotConfirm, and I'd lowercase the leading d

>+{
>+    var SelectedItems = currentView().getSelectedItems({});
I'd lowercase the leading S

>+    deleteItems( SelectedItems, DoNotConfirm );
Remove spaces around SelectedItems, DoNotConfirm

>+}
>+
>+/**
>+*  This is called from the unifinder's delete command
>+*
Remove this line ^^
>+*/
Indent the bottom 2 comment lines one space for proper javadoc style.

>+function deleteItems( SelectedItems, DoNotConfirm )
Remove spaces around SelectedItems, DoNotConfirm, and lowercase the leading chars.

>+{
>+    if (!SelectedItems)
>+        return;
Add curly braces to this if statement
>+
>+    startBatchTransaction();
>+    for (i in SelectedItems) {
I think you need |var i| to prevent strict warnings.
>+        var aOccurrence = SelectedItems[i];
>+        if (aOccurrence.parentItem != aOccurrence) {
>+            var event = aOccurrence.parentItem.clone();
>+            event.recurrenceInfo.removeOccurrenceAt(aOccurrence.recurrenceId);
>+            doTransaction('modify', event, event.calendar, aOccurrence.parentItem, null);
>+        } else {
>+            doTransaction('delete', aOccurrence, aOccurrence.calendar, null, null);
>+        }
>+    }
>+    endBatchTransaction();
>+}

Truthfully, if you wanted to rename those passed in variables aItems and aDontConfirm, I'd dig that also.  Up to you.

>+++ mozilla/calendar/resources/content/clipboard.js	2007-01-16 13:20:18.187500000 +0100
>-            var destCal = getDefaultCalendar();
>-            var firstDate = document.getElementById("view-deck").selectedPanel.selectedDay;
>+            var destCal = "ltnSelectedCalendar" in window ? ltnSelectedCalendar() : getDefaultCalendar();
This seems liek something that should be in calUtils, but either way put ( ) around the comparison, and then wrap the line in some handy way.

>+            var firstDate = currentView().selectedDay;


r=lilmatt with the style fixes.

Comment 4

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

Comment on attachment 251627 [details] [diff] [review]
patch v1

r2=mvl

Comment 5

[Michael Büttner (no reviews TFN)]

Since bug #366560 has already been checked in, I moved deleteItems() and deleteEventCommand() from calendar.js to calUtils.js instead of calendarUtils.js (see my comment #1 above).

Checked in on trunk and MOZILLA_1_8_BRANCH