Open Bug 551814 Opened 14 years ago Updated 2 years ago

"All Day Events" are sorted inconsistently

Categories

(Calendar :: Calendar Frontend, defect)

defect

Tracking

(Not tracked)

People

(Reporter: cal-bugz, Unassigned)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8) Gecko/20100202 Firefox/3.5.8 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.18pre) Gecko/20080917 Sunbird/0.9

If there are multiple "All Day" events on the same day they are consistently sorted in the same order when switching between Day, Week, Multiweek, and Month views.

Reproducible: Always

Steps to Reproduce:
1.  Enter 4 "All Day" Events on the same day.

Click "New Event" and name it "Event A" 
-> check the box "All day Event"
-> Click "Save and Close"

Repeat above for "Event B", "Event C", "Event D".

2. Now click the views Day, Week, Multiweek and Month.  Note that some views sort the events alphabetically and others do not.


Actual Results:  
On my system the events are sorted as follows.

Day View: Event C, Event A, Event B, Event D
Week View: Event C, Event A, Event B, Event D
Multiweek View: Event A, Event B, Event C, Event D
Month View: Event A, Event B, Event C, Event D


If I select "Print" I see the following in "Print Preview"

Layout List: Event B, Event A, Event C, Event D
Layout Monthly Grid: Event B, Event A, Event C, Event D
Layout Weekly Planner: Event B, Event A, Event C, Event D






Expected Results:  
I expect the events to be sorted in alphabetical order in every calendar view and in every print layout.
Version: unspecified → Sunbird 0.9
The behavior of Day View, Week View, Multiweek View and Month View appears to be fixed in Win-32 Sunbird 1.0b1.  The bug still exists for Print Preview in Sunbird 1.0b1.

I tried a nightly build of Win-32 Sunbird 1.0b2pre on Mar-15-2010 and at first glance it appears to be fixed in Print Preview as well as the calendar views.

Great!
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
I tried a nightly build of Win-32 Sunbird 1.0b2pre on Mar-31-2010 and the bug is back.

http://ftp.mozilla.org/pub/mozilla.org/calendar/sunbird/nightly/latest-comm-1.9.1/
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
Version: Sunbird 0.9 → Trunk
Attached patch I have fixed described bug. — — Splinter Review
I have fixed print view and the view in main window to be more understandable so events are sorted alphabetically as it was sorted in month and multi-week view.
Now print view and view in main window gives the same result as it should.

I am new to open-source, and I hope I fulfilled everything that needed to be done before reviewing and I hope I have send it to right address.

And a question, will I be informed whether the bug fix is accepted, and is it going to be included in nightly build?

Thank you for your time.
Aleksandar
Attachment #448088 - Flags: review?(philipp)
(In reply to comment #3)
> Created an attachment (id=448088) [details]
> I have fixed described bug.

Thanks for your patch! One question that came to my mind when having a quick look at your code: Why have you changed files specific to the calendar views and not just the ones affecting the print views?
Assignee: nobody → aleksandroz.mail
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Hi Martin,
The explanation of the bug included report that "some views sort the events alphabetically and others do not". I have changed files specific to the calendar view so every view is sorting events alphabetically ,if they are on same date. In this version of Calendar "all day" events are sorted alphabetically in Multiweek and Month view , but not alphabetically  in Day and Week view.I have changed specific files so everything is sorted the same way as it was specified in "Expected Results".
Hi Aleksander,

I am very pleased with the results of your changes.  Using the patch my events are now sorted in the same order in every "Calendar View" and also in every "Print Layout".  I hope that your changes will be approved by Mozilla and that they are soon available in a nightly build.

Thank you,

Ed
I have only done a little testing but this patch may also fix bug 373562.

Ed
Comment on attachment 448088 [details] [diff] [review]
I have fixed described bug.

>+      <!-- list exclusively used for sorting items -->
>+	  <field name="mItems">[]</field>
Do we really need this array separately? Can't you re-use mItemBoxes for this? I can imagine if you keep mItemBoxes sorted from the beginning (i.e each time an event box is added), then you don't need an extra array just for sorting. See also the below note on using binaryInsert.



>       <method name="addEvent">
>         <parameter name="aItem"/>
>         <body><![CDATA[
>+function sortByTitle(a,b){ 
>+			  // sort all day events before events with a duration
>+              if (a.item.startDate.isDate && !b.item.startDate.isDate) return -1;
>+              if (!a.item.startDate.isDate && b.item.startDate.isDate) return 1;
>+
Please check your indenting here. You also need to be using 4 spaces instead of tab characters.


>+              var cmp;
Use let instead of var.

>+              if (cmp != 0)
>+                return cmp;
Use braces even for one line ifs and 4 space indent when opening a block.


>+          this.mItems.push({ item: aItem });
>+          this.mItems.sort(sortByTitle);
Since you want to keep this array sorted, it will be faster if you use the binary insert sort algorithm. We have helpers to do this in calUtils, see http://mxr.mozilla.org/comm-central/source/calendar/base/src/calUtils.js#1812



>+          for (var i = 0; i < this.mItems.length; i++) {
>+          	if (this.mItems[i].item == aItem) {
>+          		if (this.mItems[i+1]) {
>+          			before = this.findBoxForItem(this.mItems[i+1].item);
>+          		}
>+          		break;
>+          	}
>+          }


>           for (var i in this.mItemBoxes) {
>             if (this.mItemBoxes[i].occurrence.hashId == aItem.hashId) {
>+			  for (var j = 0; j < this.mItems.length; j++) {
>+          	  	if (this.mItems[j].item == this.mItemBoxes[i].occurrence) {
>+          	  		this.mItems.splice(j, 1);
>+          	  	}
I'm a bit concerned about performance here. for loop inside a for loop...

>+function sortByTitle(a,b){ 
>+	// sort all day events before events with a duration
>+	if (a.startDate.isDate && !b.startDate.isDate) return -1;
>+	if (!a.startDate.isDate && b.startDate.isDate) return 1;
>+
>+	var cmp;
>+	cmp = a.startDate.compare(b.startDate);
>+	if (cmp != 0)
>+		return cmp;
>+
>+	cmp = a.endDate.compare(b.endDate);
>+	if (cmp != 0)
>+		return cmp;
>+		
>+	if (a.title < b.title) {
>+		return -1;
>+	}
>+	if (a.title > b.title) {
>+		return 1;
>+	}	   
>+	return 0;
>+}
Is this the same comptor you use in the views? If so then we should put this in a helper file and use it from everywhere.


r- for now to get the above fixed. Let me know if you need any help on those performance considerations.
Thanks for your effort on this patch, I really appreciate!
Attachment #448088 - Flags: review?(philipp) → review-
Whiteboard: [good first bug]
Aleksandar, any updates here? It would be very cool if you could update your patch.
Hello Aleksander, too bad we haven't heard from you. Could you at least tell me if you still plan on looking into the review comments?
Hello Philip.
Thanks you for detailed explanation I appreciate it.
I was't checking this email very often so your email got lost among many emails.

It has been some time since I created a patch here, so just wanted to ask, do you know some web-page or a document how to download latest trunk code and things needed to download a code and create a patch? I know that I have spent some amount of time looking for that information first time I got here (windows7 x64)
If not, I will try to find it.
And again, thanks for explanation, I will give it a try.

Thanks,
Aleksandar
I'm glad to hear from you :-)

You can find the guide here: https://developer.mozilla.org/En/Simple_Thunderbird_build

Note that you might have some trouble building if you have Visual Studio 2008 and not 2010, see bug 753456 for a workaround.
See also bug 481837 which asks to sort first by position in calendar list and then alphabetically.

Hi, I want to work on this bug . Can u please guide me?

Flags: needinfo?(philipp)
Keywords: good-first-bug
Whiteboard: [good first bug]
Assignee: aleksandroz.mail → nobody
No longer blocks: 373562
Status: ASSIGNED → NEW

Is this bug still available? If yes, then I would like to give it a shot

I wish to contribute to this bug. Kindly assign this to me.

Assignee: nobody → nikitasen98
Status: NEW → ASSIGNED
Flags: needinfo?(philipp)
Assignee: nikitasen98 → nobody
Status: ASSIGNED → NEW
Version: Trunk → unspecified

If possible, I would like to contribute in fixing this bug.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: