1272243 - Tasks are not ordered by start/due time in Multiweek/Month view (Lightning)

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[M Lopez-Ibanez]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160425115337

Steps to reproduce:

Create two tasks (task11 and task10) on the same day with different start dates (11.00 and 10.00).


Actual results:

 In Multiweek and Month view, task11 shows before than task10.


Expected results:

Task11 should appear after task10. Note that in Week and Day views tasks are correctly ordered.

Comment 1

[Decathlon]

Attachment: patch-v1

Confirmed.
In multiweek/month views the tasks are added without sorting by date.
As far as I can see it's always been like that, but I can't find an older bug report so I validate this one.

The patch should fix.

Comment 2

[aleth [:aleth]]

Please add commit links when you land patches.
https://hg.mozilla.org/comm-central/rev/7533feed2f235cdcde7f5adc7e729ad25d50cfce

Comment 3

[aleth [:aleth]]

Backed out for test failures:
https://hg.mozilla.org/comm-central/rev/c89aaa727a7d3ec8199ecec7834875334bd1b097

Comment 4

[Decathlon]

I was just adding the links when you posted.
I was pretty sure I had tried this on the try-server along with another patch but I was wrong.

Sorry for the extra work.

Comment 5

[Decathlon]

Attachment: patch - v2

The patch fails with test_compare_todo():

http://mxr.mozilla.org/comm-central/source/calendar/test/unit/test_view_utils.js#106

The test calls "view.compareItems" with two tasks without entry and due date but actually this condition never arises when sorting elements that have to be displayed in the view, because at least one date is required:

http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-base-view.xml#84

I've added two checks about null dates to avoid the problem.
Maybe the second check is a useless caution because Lightning always adds the end date even for the few cases considered in rfc5545 of events without DTEND (event with DTSTART + DURATION or event all-day, one day, with only DTSTART date):

http://mxr.mozilla.org/comm-central/source/calendar/base/src/calEvent.js#176


Here the try-server build:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3f4c7e1f0924aa6636e68a57f6312dc7a756cbc1&selectedJob=19323

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8762674 [details] [diff] [review]
patch - v2

Review of attachment 8762674 [details] [diff] [review]:
-----------------------------------------------------------------

r+, sorry for the delay. I think an extra null check is fine, better safe than sorry!

Note that we have landed patches to run eslint on the code. It is not (easily) runnable at the moment since I have to finish up some m-c patches, but there may be issues here due to the one-line ifs. Check the current code for details. Please email me if you have questions, I'm happy to help you set this up.

r=philipp with this minor issue:

::: calendar/base/modules/calViewUtils.jsm
@@ +42,3 @@
>          if (cmp != 0) return cmp;
>  
> +        if (!aEndDate || !bEndDate) return 0; 

Whitespace snuck in here

Comment 7

[Decathlon]

Attachment: patch - v2 (with eslint rules)

I did some hack to make ESLint working with the rules in \calendar\.eslintrc and I get in the output just a warning about the definition of the rule 'mozilla/import-globals' which was not found, so I think this patch is OK.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Thanks for going through the effort! I just landed an update to eslint on mozilla-inbound, so very soon you should be able to update mozilla-central and have it work without hacks. There is a command mach eslint, you can run mach eslint calendar for example. Right now there is still an issue with the project dir not being right so you may need to do ln -s mozilla/tools in the comm-central directory. I'll send around more instructions soon. The patch looks fine though, so nothing left to do (other than wait for someone to push) in this bug.

Comment 9

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/6d6f444d58eb6c7f893c74ea41b7ab9f96e94e46
Bug 1272243 - Tasks are not ordered by start/due time in Multiweek/Month view (Lightning). r=Philipp