Closed
Bug 405111
Opened 17 years ago
Closed 17 years ago
Hidden completed tasks are visible in task list after Startup or Reload Remote Calendars
Categories
(Calendar :: Tasks, defect)
Calendar
Tasks
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: bugzilla.i.sekler, Assigned: michael.buettner)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
6.92 KB,
patch
|
berend.cornelius09
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112211 Firefox/3.0b2pre Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.10pre) Gecko/20071123 Calendar/0.8pre If the checkbox "Hide Completed Tasks" is checked, reloading remote calendars causes the completed tasks to appear in the todo list while the checkbox remains checked. To hide the completed tasks again, unchecking and checking the checkbox is required. Reproducible: Always Steps to Reproduce: 1. Open an *.ics file or add a remote calendar 2. check the "Hide Completed Tasks" checkbox 3. Reload remote calendars Actual Results: Completed tasks are shown again, the checkbox "Hide Completed Tasks" remains checked. Expected Results: Completed tasks remain hidden. It is a recent regression. Works in a 2007-11-21 build (checkout finish shortly before 02:00 PST), fails 2007-11-22 (checkout finish shortly before 10:00 PST).
Reporter | ||
Comment 1•17 years ago
|
||
Caused by the checkin in Bug 253396?
Reporter | ||
Comment 2•17 years ago
|
||
Happens as expected also on Windows and on Linux with trunk builds as well, changing OS to "All". Mozilla 1.8 Branch + Trunk.
OS: Linux → All
Comment 3•17 years ago
|
||
Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.10pre) Gecko/20071126 Calendar/0.8pre. The following is broken with "Hide completed tasks" active: - reload calendars makes the completed tasks visible again - not as expected - after startup the completed tasks are visible - not as expected - marking a task as completed doesn't remove it from the list - not as expected Only the task list is affected. The calendar view work fine. Regression Range: WORKS in Sunbird 0.8pre (2007-11-21-05) FAILS in Sunbird 0.8pre (2007-11-22-05) Checkins during regression range: http://tinyurl.com/32ko8c Seems to be caused by the checkin for Bug 253396.
Blocks: 253396
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Hardware: PC → All
Summary: "Reload remote calendars" unhides completed tasks → "Reload remote calendars" unhides completed tasks in task list
Version: unspecified → Mozilla 1.8 Branch
Updated•17 years ago
|
Summary: "Reload remote calendars" unhides completed tasks in task list → Hidden completed tasks are visible in task list after Startup resp. Reload Remote Calendars
Comment 4•17 years ago
|
||
Major regression and feature lost compared to Sunbird 0.7. In my opinion this should be fixed for 0.8.
Flags: wanted-calendar0.8?
Summary: Hidden completed tasks are visible in task list after Startup resp. Reload Remote Calendars → Hidden completed tasks are visible in task list after Startup or Reload Remote Calendars
Updated•17 years ago
|
Flags: wanted-calendar0.8? → wanted-calendar0.8+
Assignee | ||
Comment 6•17 years ago
|
||
This patch should do the trick...
Assignee: nobody → michael.buettner
Status: NEW → ASSIGNED
Attachment #292076 -
Flags: review?(Berend.Cornelius)
Comment 7•17 years ago
|
||
As far as I tested this patch it works fine when you remove or add calendars but the testcase mentioned in comment #3: > - marking a task as completed doesn't remove it from the list - not as expected. is not covered by this patch. The functions "addItem" and "modifyItem" do not consider the state of the checkbox "Hide Completed Task". As the initial description of this patch is resolved by your patch I leave it up to you if you want to fix this within this issue or if you want to set up a follow up. The implementation of the patch is good. I would consolidate the assignment of the filters or probably even better consolidate the implementation of the methods "onCalendarAdded" and "Refresh". As far as I can see both methods do a refresh - one method on a single calendar and one on the composite calendar.
Updated•17 years ago
|
Attachment #292076 -
Flags: review?(Berend.Cornelius) → review+
Assignee | ||
Comment 8•17 years ago
|
||
> The functions "addItem" and "modifyItem" do not consider the state of > the checkbox "Hide Completed Task". Good catch. This version of the patch adds the necessary bits and pieces. Re-requesting review since this in an integral part of this patch. > I would consolidate the assignment of the filters or probably even > better consolidate the implementation of the methods "onCalendarAdded" > and "Refresh". As far as I can see both methods do a refresh - one method > on a single calendar and one on the composite calendar. onCalendarAdded() issues addItem() calls, therefore adds items but leaves the existing ones intact. refresh() collects all items and completely replaces the existing array. We could factor this out in a separate method, but I hardly believe this buys us any benefit. But I'm open for suggestions on how you'd do that...
Attachment #292076 -
Attachment is obsolete: true
Attachment #292934 -
Flags: review?(Berend.Cornelius)
Comment 9•17 years ago
|
||
When I tested this patch I still found that modifying a task is not possible.
Removing the completed state from a task when the "Hide Completed Tasks" checkbox is checked removes another task from the tree.
I modified the function modifyItem a bit and found that the following code works well.
Maybe you can try this:
> modifyItem: function tTV_modifyItem(aNewItem, aOldItem) {
> var index = this.binding.mHash2Index[aOldItem.hashId];
> if (index != undefined) {
> if (aNewItem.isCompleted != aOldItem.isCompleted) {
> if (aNewItem.isCompleted && this.binding.hideCompleted) {
> this.removeItem(aNewItem);
> delete this.binding.mHash2Index[aOldItem.hashId];
> return;
> }
> }
> delete this.binding.mHash2Index[aOldItem.hashId];
> this.binding.mHash2Index[aNewItem.hashId] = index;
> this.binding.mTaskArray[index] = aNewItem;
> this.tree.view.selection.select(index);
> this.treebox.invalidateRow(index);
> }
> },
Updated•17 years ago
|
Attachment #292934 -
Flags: review?(Berend.Cornelius) → review+
Assignee | ||
Comment 10•17 years ago
|
||
Berend, thanks for spotting the missing return statement. Deleting the key from the hash table isn't necessary, since removeItem() rebuilds the table from scratch. I change the patch accordingly before checking it in.
Assignee | ||
Comment 11•17 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Target Milestone: --- → 0.8
Version: Mozilla 1.8 Branch → unspecified
Comment 12•16 years ago
|
||
Verified with Lightning 2008012900 and Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.12pre) Gecko/20080127 Calendar/0.8pre
Status: RESOLVED → VERIFIED
Comment 13•16 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.9) Gecko/20071031 Lightning/0.8pre (2008012818) Thunderbird/2.0.0.9 ID:2007103104 VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•