Closed Bug 351880 Opened 18 years ago Closed 16 years ago

Selecting repeating event in Unifinder does not select events in Main View

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sebo.moz, Assigned: gekacheka)

References

Details

Attachments

(1 file, 1 obsolete file)

Selecting repeating event in Unifinder does not select events in Main View.

Steps to reproduce:
1. Create repeating event
2. Select this event in Unifinder
Result: unlike normal events, this does not highlight the repeating events

Expected behaviour: highlight all events associated to this parent item.

See bug 350852 for the reverse procedure.

done using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060908 Calendar/0.3a2+
The bug is present with non repeating event two... but it can be reproduce only with week or day view. It works with multiweek or month view.

If you select more than one event in the unifinder it works
It could be great to have this bug fixed for 0.9?
(patch -l -p 1 -i file.patch)

This patch is for multiday-view only.

Problem is that multiday-view.setSelectedItems looks for columns to inform, but only looks for columns of the item, not all its occurrences.

Solution is to inform all columns of all occurrences in view.

(this patch depends on patch for bug 405196 so hiliting in multiday view when selected from unifinder works)
Assignee: nobody → gekacheka
Attachment #340860 - Flags: review?(Berend.Cornelius)
Depends on: 405196
i've applied the patches... it works well for me!!!
Great work
I've just a little problem...

Step to reproduce :

Click on an item in the unifinder :  it is highlighted well in the calendar view...

But click on another item in the calendar view... the new item is highlighted but the old one is still highlighted... this is a problem
(patch -l -p 1 -i file.patch)

Improved so if a recurrent parent item is selected, all its occurrences in view are selected in columns (rather than passing parent item as an occurrence to columns).  (Depends on bug 405196 patch v2.)

(Merci! for quick testing, Hubert.)
Attachment #340860 - Attachment is obsolete: true
Attachment #340909 - Flags: review?(Berend.Cornelius)
Attachment #340860 - Flags: review?(Berend.Cornelius)
There's still a problem with deselection :
Events outside the actual view are not unselected

Step to reproduce :
* select an event ... using the unifinder (or click on the event)
* go on the previous week
* select an event ... using the unifinder (or click on the event)
* go on the next week
Look the event is still selected... and if you select another event... you've got two selected events...
Good testing, Hubert, merci!

Forgetting old selection, when navigating to new date, 
fixed by bug 405196 patch v3.
Another problems :

* Single selection :

 * Select an event by clicking on it (not in the unifinder)
 * Move It
 * Select the same event
 * Move It...
 * Now it will remain selected for ever...

 * Select an event by clicking on it (not in the unifinder)
 * It appears selected in the unifinder
 * Change week... it is unselected in the unifinder

* Multiple selection
 
 * MultiSelect  a single event and an occurence of a recurring event (with CTRL)
 * The unifinder shows random multiple selection
Merci again for excellent testing, Herbert!

Single selection problems fixed in bug 405196 patch v4.

Grid -> Unifinder recurring event selection problems left for bug 350852.
Your fix seems ok now...

but i found another problem with resize/ drag'n drop if you have time take a look at : https://bugzilla.mozilla.org/show_bug.cgi?id=457854
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: PC → All
I've still a little annoying problem with selection :

Step to reproduce :

1. Select an event by directly clicking on it
2. Create an event by mouse (drag'n drop)
3. The previous event is no more selected, and a random event is selected instead

It should be better if the created event get selected
in reply to comment 13

I have not been able to reproduce this reliably on w2k --- out of 20 or 30 tries it might have happened once. 

I suggest starting a new bug for it.  (Also, if you can, see if you can find steps that reproduce the problem starting say from a blank local calendar.  It might be dependent on calendar size, or network slowness if it is remote.)

(FYI, I'll be away and won't be able to work on it in the near future.  Thank you again for your thorough testing of my patches!)
There are a lot of nested for loops in this patch. Is there a way we could reduce these? Maybe by optimizing findColumsnForItem for this specifc use case? Also maybe findColumsnForOccurrences (or hashing techniques used within) helps to improve performance.

I must admit I haven't tested this patch, but it kind of scares me to see 3 nested levels of for each loops with calls to functions that use loops themselves.
This is multiday view, so there are at most 7 occurrences in view (usually just 1) and at most 7 columns per occurrence (usually just 1 unless an event overlaps midnite), so there are low constant bounds.

findColumnsForOccurrences is not really useful here, because it would mix all the occurrences' columns in one set and there would still have to be some way to figure out which occurrence is in which columns for column.selectOccurrence.
Comment on attachment 340909 [details] [diff] [review]
patch v2: multiday view select -- find columns for all item occurrences

I think that performance-wise the patch is Ok. I wouldn't know how to make it better either. We definitely have a performance issue with the multiday-view but this is mainly relevant for the code that is invoked by the "relayout" method which is not the case her.
But altough the patch looks good, I still can't see any highlighting on the current version of the trunk codeline, so I have to deny the review.

I think you could consolidate the methods "unselectOccurrences" and "selectOccurrences" and probably the according loops in the method "setSelectedItems" where the items are selected or unselected.
The method "getItemOccurrencesInView" is only used internally and could therefor be named "_getItemOccurrencesInView"
Attachment #340909 - Flags: review?(Berend.Cornelius) → review-
I just see that the highlighting issue has been dealt with in  Bug 405196 -  unifinder: selecting event in unifinder doesn't select it in day or week view, so I probably have to withdraw my review-denial.
Comment on attachment 340909 [details] [diff] [review]
patch v2: multiday view select -- find columns for all item occurrences

The patch works fine with consideration to the dependency to Bug 351880 -  Selecting repeating event in Unifinder does not select events in Main View
r=berend. 
Maybe you are ambitious enough to add my proposed changes anyway. calendar-multiday-view.xml  has become quite a monster and needs some polishing in many places.
I will leave the bug open for that.
Attachment #340909 - Flags: review- → review+
patch pushed to comm-central
http://hg.mozilla.org/comm-central/rev/94038cf02f33
Target Milestone: --- → 1.0
(In reply to comment #19)
> (From update of attachment 340909 [details] [diff] [review])
> Maybe you are ambitious enough to add my proposed changes anyway.
> calendar-multiday-view.xml  has become quite a monster and needs some polishing
> in many places.
> I will leave the bug open for that.

Resolving FIXED. For clean-up work and refactoring new bugs should be filed!
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Checked in lightning and sunbird build 20081221 -> VERIFIED.
Status: RESOLVED → VERIFIED
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: