Last Comment Bug 373251 - Can open multiple windows of a single event/task from Agenda, Task lists and Unifinder
: Can open multiple windows of a single event/task from Agenda, Task lists and ...
Status: RESOLVED FIXED
[needed beta][no l10n impact]
: relnote
Product: Calendar
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: 1.0b7
Assigned To: PMARTINAK
:
Mentors:
: 473746 475508 580653 648057 (view as bug list)
Depends on: 390293
Blocks: 687286
  Show dependency treegraph
 
Reported: 2007-03-08 15:58 PST by lithis
Modified: 2011-10-01 02:22 PDT (History)
8 users (show)
philipp: blocking‑calendar1.0+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch that search for already open item window (5.21 KB, patch)
2011-08-10 06:40 PDT, PMARTINAK
philipp: review+
Details | Diff | Splinter Review

Description lithis 2007-03-08 15:58:52 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a3pre) Gecko/20070307 Calendar/0.6a1

If an event is opened from the event list, it opens a new window regardless of whether that event was already open.

Reproducible: Always

Steps to Reproduce:
1. Double-click on an event in the event list.
2. Leave the event window open and return to the main calendar window.
3. Double-click on the same event again.
Actual Results:  
A second, identical "Edit Event" dialog is opened.

Expected Results:  
Only one "Edit Event" dialog should appear.
Comment 1 Omar Bajraszewski 2007-03-09 10:58:36 PST
It's the same story like in bug 373250. These bugs are very similar. I agree that only one edit event dialog should appear.
Comment 2 Bas van den Bosch 2007-03-10 07:26:54 PST
Apart from the behaviour described in bug 373250, I can't confirm the behaviour of two windows opening. So WFM for this bug in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3pre) Gecko/20070305 Calendar/0.5pre

Can someone confirm the behaviour in 0.6a1?
Comment 3 Martin Schröder [:mschroeder] 2007-03-10 07:40:07 PST
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3pre) Gecko/20070308 Calendar/0.5pre.

Bas, have you really tried to open the same event multiple times from the unifinder (event list)?
Comment 4 Bas van den Bosch 2007-03-12 06:12:54 PDT
I stand corrected, when opening by doubleclicking on the unifinder-list it opens two windows, when doubleclicking on the calendar-view, the "edit Event" title changes to "New Event" like in previously mentioned bug. There should be a check on the open dialogs to see wether the event is already open. 
Comment 5 Martin Schröder [:mschroeder] 2007-03-16 04:35:48 PDT
This is also true for Agenda and Todo panel in Sunbird & Lightning.
Comment 6 lithis 2007-05-07 20:48:22 PDT
would this be considered a dataloss bug?

steps to reproduce:
1. an event is opened.
2. the same event is opened again; a second window opens.
3. changes are made to the event in one of the windows.
4. the ok button in the window with the changes is clicked.
5. the ok button in the other window is clicked.

actual results:
the changes would be overwritten when the unchanged window is closed.

expected results:
only one window should exist.

i would also like to point out that although double-clicking on the calendar view only opens one instance of the "edit event" window, it is a different instance than the windows that open from the event list (unifinder).

steps to reproduce:
1. open an event from the event list; an "edit event" window opens.
2. open the same event from the calendar view.

actual results:
a second "edit event" window opens.

expected results:
again, only one window should exist.
Comment 7 Michael Büttner (no reviews TFN) 2007-05-08 00:49:04 PDT
When we made the event dialog window modeless I also added code that remembers which items are currently "open for editing" and taking care if further modification requests should come along. The part that should handle this can be found at [1]. Probably it has something to do with finding the "already open modification" in the list.

[1] http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-views.js#121
Comment 8 lithis 2007-05-09 15:10:28 PDT
(In reply to comment #7)
> When we made the event dialog window modeless I also added code that remembers
> which items are currently "open for editing" and taking care if further
> modification requests should come along.

are you talking about inline edits in the calendar view? (if not, i'm sorry; i know nothing about the mozilla source code.) this bug has nothing to do with that; it's about multiple concurrent windows for the same event.
Comment 9 Michael Büttner (no reviews TFN) 2007-05-10 05:13:27 PDT
(In reply to comment #8)
> are you talking about inline edits in the calendar view? (if not, i'm sorry; i
> know nothing about the mozilla source code.) this bug has nothing to do with
> that; it's about multiple concurrent windows for the same event.
no, i'm not talking about inline editing, we're indeed talking about the very same feature :-) don't mind, i just wanted to point to the relevant source code location.
Comment 10 Simon Paquet [:sipaq] 2007-11-24 12:20:06 PST
Setting wanted0.8- as the main Calendar developers will not devote any time to
this in the 0.8 timeframe. Patches are, of course, always welcome.
Comment 11 Stefan Sitter 2009-01-15 05:17:08 PST
*** Bug 473746 has been marked as a duplicate of this bug. ***
Comment 12 Andreas Treumann 2009-01-27 03:26:03 PST
*** Bug 475508 has been marked as a duplicate of this bug. ***
Comment 13 Stefan Sitter 2011-04-06 11:53:45 PDT
*** Bug 648057 has been marked as a duplicate of this bug. ***
Comment 14 Philipp Kewisch [:Fallen] 2011-05-02 09:20:34 PDT
*** Bug 580653 has been marked as a duplicate of this bug. ***
Comment 15 Philipp Kewisch [:Fallen] 2011-07-17 00:17:52 PDT
This could be fixed together with bug 390293 by creating a service that takes care of opening event dialogs and also the actual item modification.
Comment 16 PMARTINAK 2011-08-10 06:40:33 PDT
Created attachment 552065 [details] [diff] [review]
Patch that search for already open item window

This patch avoids opening multiple editor windows for an item. 

It works when the user opens an item from the list of events or tasks, or from the calendar/task view s(context command or double-click). 

calFindItemWindow (calUtils.js) function allows you to find the window of an element from its ID. 
If a window exists, it is highlighted. 

Without this patch, double-clicking twice on the calendar view closes existing window and saves item without notification. A new instance is created (see caldav log in console).
Comment 17 Philipp Kewisch [:Fallen] 2011-08-24 01:20:10 PDT
Comment on attachment 552065 [details] [diff] [review]
Patch that search for already open item window

Nice, much simpler than I had in mind! I was working on making a dialog service that put everything into an independent xpcom component, but this is much easier to handle. r=philipp with some style stuff cleaned up, I'll take care of that before checkin.
Comment 18 Philipp Kewisch [:Fallen] 2011-08-24 01:40:24 PDT
Comment on attachment 552065 [details] [diff] [review]
Patch that search for already open item window


>     modifyOccurrence: function (aOccurrence, aNewStartTime, aNewEndTime, aNewTitle) {
> 
>+        var dlg=calFindItemWindow(aOccurrence);
>+        if (null!=dlg){
>+          dlg.focus();
>+          return;
>+        }
>+        
>         aOccurrence = this.finalizePendingModification(aOccurrence);

Hmm actually I'm not sure if this is good here, but I do see why you did it. If left in, then it will be impossible to move an event in the view via click'n'drag when an event dialog with that item is still open. If taken out, then clicking on the same event more than once will cause a modification failed message due to the way finalizePendingModification works.

Putting back on my review queue so I don't forget this.
Comment 19 Bas van den Bosch 2011-08-24 02:06:08 PDT
@18: "If left in, then it will be impossible to move an event in the view via click'n'drag when an event dialog with that item is still open."

This is a good thing, dragging is also modifying an event, we don't want events being modified when a dialog is open.
Comment 20 Philipp Kewisch [:Fallen] 2011-08-24 04:10:03 PDT
Yes, but don't you think it might confuse the user that he can't drag the event?
Comment 21 Bas van den Bosch 2011-08-24 05:21:31 PDT
I don't think so. If, with an open dialog, an event is dragged and the data in the dialog is saved after that, the event will be moved back (assuming no altered times in the dialog) which will be more confusing imho. You could throw a warning when trying to move an event which is already open.
Comment 22 Daniel 2011-08-29 14:53:21 PDT
Does this patch also help when you open a Task from within a Reminder and closing the changed Reminder before the changed Task? 

(In reply to PMARTINAK from comment #16)
> Created attachment 552065 [details] [diff] [review]
> Patch that search for already open item window
> 
> This patch avoids opening multiple editor windows for an item. 
> 
> It works when the user opens an item from the list of events or tasks, or
> from the calendar/task view s(context command or double-click). 
> 
> calFindItemWindow (calUtils.js) function allows you to find the window of an
> element from its ID. 
> If a window exists, it is highlighted. 
> 
> Without this patch, double-clicking twice on the calendar view closes
> existing window and saves item without notification. A new instance is
> created (see caldav log in console).
Comment 23 Philipp Kewisch [:Fallen] 2011-08-29 23:16:06 PDT
(In reply to Daniel from comment #22)
> Does this patch also help when you open a Task from within a Reminder and
> closing the changed Reminder before the changed Task? 

Good question, I haven't tried that. I don't think it will though, since the reminders don't use the standard mechanisms. If we solve that using this patch, then you won't be able to dismiss a reminder before you close the event dialog, which seems wrong to me.

Maybe we should solve this the other way around: If the item gets changed from outside the event dialog, the event dialog should show a bar telling the user that the item was changed while viewing it. He would then get the option to either reload the event and discard changes, or to ignore and write anyway. The latter option would then update the old item so that the write succeeds.
Comment 24 Philipp Kewisch [:Fallen] 2011-09-17 14:34:26 PDT
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/31a247f6426c>
-> FIXED
Comment 25 Philipp Kewisch [:Fallen] 2011-09-17 14:41:34 PDT
Backported to comm-aurora <http://hg.mozilla.org/releases/comm-aurora/rev/a7f24c946128>
Comment 26 Philipp Kewisch [:Fallen] 2011-09-17 14:42:32 PDT
Backported to comm-beta <http://hg.mozilla.org/releases/comm-beta/rev/d030179bc5a4>
Comment 27 Martin Schröder [:mschroeder] 2011-09-30 12:01:13 PDT
Checked in without r+?
Comment 28 Philipp Kewisch [:Fallen] 2011-10-01 00:47:41 PDT
Comment on attachment 552065 [details] [diff] [review]
Patch that search for already open item window

Thanks, r=philipp
Comment 29 Philipp Kewisch [:Fallen] 2011-10-01 02:22:21 PDT
Oh right, reasoning: I think for 1.0b7 and maybe even 1.0 we should take this patch. Then later on we fix bug 687286 to turn around the logic.

Note You need to log in before you can comment on or make changes to this bug.