359443 - Deleting item from Agenda deletes an email instead

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[ken]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

select an agenda item, press delete. previously selected email disappears.


Reproducible: Always

Steps to Reproduce:
1. select an agenda item
2. press delete. 
2.
3.




lightning 0.3 
thunderbird 1.5.0.7

Comment 1

[Stefan Sitter [:ssitter]]

Confirmed using Thunderbird/1.5.0.8 (20061025) and Lightning/0.4a1 (2006111006) on Windows 2000.

Probably the Agenda tab needs a similar fix as done with Bug 353470 for the Todo tab.

Comment 2

[John Bäckstrand]

I see this in Thunderbird 2.0b1 ID:2006120404 (nightly) and Lightning build 2006120403, only mail gets deleted, while item stays. The title here seems to imply that item gets deleted.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

This is serious bad news.  Adjusted the title.

Comment 4

[Stefan Sitter [:ssitter]]

*** Bug 364853 has been marked as a duplicate of this bug. ***

Comment 5

[Clas johnny]

There should also be a regret opportunity to undo the last action.

Comment 6

[Stefan Sitter [:ssitter]]

Issue still exists with the new Today Pane using Lightning 0.7pre (2007072308).

Comment 7

[Daniel Boelzle [:dbo]]

Berend volunteered

Comment 8

[Berend Cornelius]

Attachment: patch to be reviewed

This patch takes care that the keypressed-event is handled by the agenda-pane

Comment 9

[Michael Büttner (no reviews TFN)]

Comment on attachment 281185 [details] [diff] [review]
patch to be reviewed

>+agendaTreeView.onKeyPress =
>+function onKeyPress(event)
>+{
>+  const kKE = Ci.nsIDOMKeyEvent;
>+  switch(event.keyCode) {
>+    case kKE.DOM_VK_DELETE:
>+      document.getElementById('agenda_new_event_command').doCommand();
>+      this.deleteEvent();
>+      event.preventDefault();
>+      event.stopPropagation();
>+      break;
>+  }
>+};
First of all, even with this patch in place the email gets nuked when hitting the 'delete'-key. You should switch the above lines 'preventDefault()' and 'stopPropagation()' to make it work. Also, please remove the 'deleteEvent()' call, it seems to be superfluous. Furthermore, the command is named 'agenda_new_event_command' but it deletes events, this is probably a copy'n'paste bug.

>+   var selectedItems = new Array();
>+   var tree = document.getElementById("agenda-tree");
>+   var start = new Object();
>+   var end = new Object();
>+   var numRanges = tree.view.selection.getRangeCount();
>+   var t;
>+   var v;
>+   var agendaItem;
>+   for (t = 0; t < numRanges; t++) {
>+      tree.view.selection.getRangeAt(t, start, end);
>+      for (v = start.value; v <= end.value; v++) {
>+        agendaItem = this.events[v];
>+        selectedItems.push(agendaItem);
>+      }
>+   }
I suggest to introduce variables as late as possible. You could remove the declaration of the variables 't', 'v' and 'agendaItem' and write the rest as follows:
   for (var t = 0; t < numRanges; t++) {
      tree.view.selection.getRangeAt(t, start, end);
      for (var v = start.value; v <= end.value; v++) {
        selectedItems.push(this.events[v]);
      }
   }

r=mickey with the above mentioned issues being addressed. Thanks for the patch, Berend.

Comment 10

[Berend Cornelius]

patch checked in on trunk and MOZILLA_1_8_BRANCH

Comment 11

[Berend Cornelius]

Attachment: modified last patch

modified the last patch as mickey suggested

Comment 12

[Martin Schröder [:mschroeder]]

(In reply to comment #9)
> Furthermore, the command is named
> 'agenda_new_event_command' but it deletes events, this is probably a
> copy'n'paste bug.

... which wasn't fixed before checkin. :-(

Comment 13

[Michael Büttner (no reviews TFN)]

(In reply to comment #12)
> (In reply to comment #9)
> > Furthermore, the command is named
> > 'agenda_new_event_command' but it deletes events, this is probably a
> > copy'n'paste bug.
> 
> ... which wasn't fixed before checkin. :-(
> 
Berend, please change the name accordingly and check it in. It's not a major problem but just adds unnecessarily confusion. Please mention in the check-in comment that it is an add-on patch.

Comment 14

[Berend Cornelius]

Attachment: resolved the naming issue

Checked in remaining naming issue
patch checked in on trunk and MOZILLA_1_8_BRANCH.

Comment 15

[Omar Bajraszewski]

Verified with lightning 2007092403. However it's not possible to undo the delete from agenda in mail mode

Comment 16

[Martin Schröder [:mschroeder]]

(In reply to comment #15)
> Verified with lightning 2007092403. However it's not possible to undo the
> delete from agenda in mail mode
> 

Omar, please file a new bug for this issue.

Comment 17

[Omar Bajraszewski]

I've filled a new bug 398405(In reply to comment #16)
> (In reply to comment #15)
> > Verified with lightning 2007092403. However it's not possible to undo the
> > delete from agenda in mail mode
> > 
> 
> Omar, please file a new bug for this issue.
> 

I've filled a new bug 398405