Closed Bug 403061 Opened 17 years ago Closed 17 years ago

Unifinder: Edit and Delete selected event via keyboard is broken

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssitter, Assigned: ssitter)

Details

Attachments

(1 file, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.10pre) Gecko/20071108 Calendar/0.8pre

Unifinder: Edit and Delete selected event via keyboard is broken

Steps to reproduce:
1. Select an event in the unifinder
2. Press Return key to edit the selected event
3. Press Delete key to delete the selected event

Actual results:
Nothing happens. Neither the Edit Event dialog is shown nor the event is deleted. Errors in console:

after step 2:
    Error: getSelectedItems is not defined
    Source File: chrome://calendar/content/calendar-unifinder.js
    Line: 784

after step 3:
    Error: deleteEventCommand is not defined
    Source File: chrome://calendar/content/calendar-unifinder.js
    Line: 788
Flags: blocking-calendar0.8+
At least the broken delete command is caused by Bug 390508. This also broke the Cut command in the view context menu.

The edit command seems to be broken longer because the same error already occurs in Sunbird 0.7.
Assignee: nobody → ssitter
OS: Windows 2000 → All
Hardware: PC → All
Attached patch fix (obsolete) — Splinter Review
Fixes the issues mentioned above. If more than one event is selected pressing Return key will do nothing. Otherwise the Edit Event dialog will be shown. If a repeating event is selected for editing either via Return key or double click the 'All' vs. 'This occurrence' dialog will be shown. Please note that this will only work if the unifinder filter is not set to "All" or "All future" due to open unifinder bugs.
Attachment #288370 - Flags: review?(philipp)
Comment on attachment 288370 [details] [diff] [review]
fix

I'm not sure this is the wanted behavior. I'd personally prefer to edit at least one event when multiple events are selected. Asking Christian for ui-r.

If Christian agrees to your proposal, r+ for the code too.
Attachment #288370 - Flags: ui-review?(christian.jansen)
Attachment #288370 - Flags: review?(philipp)
Attachment #288370 - Flags: review+
Thanks for the patch, it heads definitely into the right direction. Keyboard support is very important.

ui+ with the following changes:

Single Selection:
 - DEL should Delete the event
 - RETURN should open the event for editing

Multi Selection
 - DEL should delete the selected events
   * UNDO should restore all previously deleted events
 - RETURN should open ALL selected events for editing

This should work in all modes (All Events, All Future, ...) If this needs to be fixed in a spin-off bug ok, but it should be fixed before we release 0.8.
(In reply to comment #4)
>  - RETURN should open ALL selected events for editing
This could bring up a tremendous amount of 'edit event' windows. Are you sure that this is the right thing to do? Or should we somehow enhance the dialog to modify several items at once?
Ok, it seems that pressing the Edit toolbar button if multiple items are selected  just opens the Edit dialog for the first item. 

I would be OK to apply the same behavior to the unifinder. At the moment I only want to fix the regression but do not add new features. A follow up bug should be filed if this behavior needs to be improved system wide (if not already existent). 

For unifinder issues regarding "All ..." filter and repeating events see Bug 310853.
(In reply to comment #5)
> (In reply to comment #4)
> >  - RETURN should open ALL selected events for editing
> This could bring up a tremendous amount of 'edit event' windows. Are you sure
> that this is the right thing to do? Or should we somehow enhance the dialog to
> modify several items at once?
> 
I aware that this could cause a tremendous amount of edit windows & and that it can take a tremendous amount of time to open them. IMO we should behave in a same way like the mail mode does. "Same things" should behave same...
Status: NEW → ASSIGNED
Attached patch fix, v2Splinter Review
Fix the regression by applying the same behavior used in the the rest of Sunbird.
Attachment #288370 - Attachment is obsolete: true
Attachment #290093 - Flags: review?(philipp)
Attachment #288370 - Flags: ui-review?(christian.jansen)
Comment on attachment 290093 [details] [diff] [review]
fix, v2

r=philipp
Attachment #290093 - Flags: review?(philipp) → review+
Keywords: checkin-needed
Checked in on HEAD and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.11pre) Gecko/20071201 Calendar/0.8pre and Lt 2007120104
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.