Closed Bug 367110 Opened 13 years ago Closed 13 years ago

Missing Thunderbird integration for copy/paste

Categories

(Calendar :: Lightning Only, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: michael.buettner, Assigned: michael.buettner)

Details

Attachments

(1 file)

Lightning's copy/paste functionality is currently not integrated into Thunderbird. This bug is dedicated to provide the missing bits and pieces.
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.
Attached patch patch v1Splinter Review
this patch implements the bits and pieces as described in above comment.
Attachment #251627 - Flags: first-review?(lilmatt)
Component: General → Lightning Only
QA Contact: general → lightning
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.
Attachment #251627 - Flags: first-review?(lilmatt) → first-review+
Attachment #251627 - Flags: second-review?(mvl)
Comment on attachment 251627 [details] [diff] [review]
patch v1

r2=mvl
Attachment #251627 - Flags: second-review?(mvl) → second-review+
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
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.