Closed Bug 405196 Opened 15 years ago Closed 14 years ago

unifinder: selecting event in unifinder doesn't select it in day or week view

Categories

(Calendar :: Calendar Frontend, defect)

Mozilla 1.8 Branch
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: aryx, Assigned: gekacheka)

References

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

Attached file testcase
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.10pre) Gecko/20071123 Calendar/0.8pre

unifinder: selecting event in unifinder doesn't select it in calendar pane
calendar: ics

Reproducibility: Often
Attachment #290000 - Attachment mime type: text/ics → text/plain
Testcase works for me using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.12pre) Gecko/20071226 Calendar/0.8pre.

Related to Bug 351880?
Depends on view --- updated summary.

Doesn't work for me with day or week view, even for single events.
(It might be selected, but the appearance is not updated so selected events are not highlighted in tan.)
Summary: unifinder: selecting event in unifinder doesn't select it in calendar pane → unifinder: selecting event in unifinder doesn't select it in day or week view
(patch -l -p 1 -i file.patch)

Problem was that calendar-event-column.selectOccurrence calls findChunkForItem, but the relayout hasn't created the chunkBoxes yet.

Solution is to keep track of the selected items so when relayout does create the chunkBoxes, it can set the selected attribute as necessary.

The selected items in column are tracked using item.hashId as keys in a hash table object.

(If getting the hashId or looking it up in the hash table turns out to be a performance bottleneck, relayout might be improved by also keeping a count of the number of selected items and avoiding the hash lookup in relayout when it is zero.  But until then it is simpler without it.)
Attachment #340856 - Flags: review?(Berend.Cornelius)
Blocks: 351880
(patch -l -p 1 -i file.patch)

Two changes:  
- don't check for selected parent item in column relayout, will always select all its occurrences in view if parentItem is selected (bug 351880)
- must use chunkbox.selected property, not set/remove "selected" attribute, otherwise chunkbox.selected fails (as currently implemented in calendar-view-core, it duplicates state in a field which is not set if only the attribute is set)
Assignee: nobody → gekacheka
Attachment #340856 - Attachment is obsolete: true
Attachment #340907 - Flags: review?(Berend.Cornelius)
Attachment #340856 - Flags: review?(Berend.Cornelius)
(patch -l -p 1 -i file.patch)

Adds fix for problem where columns did not forget selected occurrences when navigating to a different week, changing selection, and navigating back (bug 351880 comment 8, thanks Hubert). 

Fix works by clearing selection before changing column dates and restoring selection afterward, in setDateRange and setDateList.  (This is a simple direct fix, but if it turns out to be too slow it may be possible to add methods to clear column selection faster in the future.)
Attachment #340907 - Attachment is obsolete: true
Attachment #340938 - Flags: review?(Berend.Cornelius)
Attachment #340907 - Flags: review?(Berend.Cornelius)
(patch -l -p 1 -i file.patch)

Changes from patch v3:

Problem:  change week deselects in unifinder (bug 351880 comment 10, thanks Hubert!)
Fix: in setDateRange and setDateList, add suppressEvent to setSelectedEvents 
since for internal effects only, immediately restored.

Problem: dragging selected event leads to permanent select highlight
Problem: delete followed by undo leads to permanent select highlight
Fix: internalDelete must delete mSelectedItemIds[occ.hashId];
Attachment #340938 - Attachment is obsolete: true
Attachment #341095 - Flags: review?(Berend.Cornelius)
Attachment #340938 - Flags: review?(Berend.Cornelius)
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 341095 [details] [diff] [review]
patch v4: multiday view column -- store selected item ids, then set selected attribute during relayout; clear selection in setDateRange and setDateList

patch looks good and works fine as well.
r=berend
Attachment #341095 - Flags: review?(Berend.Cornelius) → review+
patch pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/aadff0fd8484
->fixed.
Thank you for your contribution!
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
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.