932217 - Update ical.js to latest version

Status: RESOLVED FIXED

(Calendar :: ICAL.js Integration, defect)

Description

[Markus Adrario [:Taraman]]

Since there have been some bugfixes recently, we should update the version of ical.js calendar is using.

Comment 1

[Markus Adrario [:Taraman]]

Attachment: update_icaljs.diff

This patch incorporates the latest ical.js from Github.

The Diff shows the additions from 4 merges on github [1-4]

This change:
> -ICAL.foldLength = 74;
> +ICAL.foldLength = 75;
I don't understand and took the value from github.

The next line is:
> ICAL.debug = false;
and I left it as it is in comm-central.

In line 4447 on comm-Central we have a change from:
> var dictParts = dict.parts = {};
to
> var dictParts = dict.parts = Object.create(null);
which I could not find in any change history on github.

Since I am unable to run xpcshell-tests at the moment, I could not test it up to now and will do some manual tests the next days.
Report on this to follow.

[1]: https://github.com/mozilla-comm/ical.js/pull/96/files
[2]: https://github.com/mozilla-comm/ical.js/pull/89/files
[3]: https://github.com/mozilla-comm/ical.js/pull/90/files
[4]: https://github.com/mozilla-comm/ical.js/pull/98/files

Comment 2

[Markus Adrario [:Taraman]]

xpcshell-tests run fine.

result without the patch:
INFO | Result summary:
INFO | Passed: 31
INFO | Failed: 4
INFO | Todo: 0
INFO | Retried: 0

result with the new version patch applied:
INFO | Result summary:
INFO | Passed: 32
INFO | Failed: 3
INFO | Todo: 0
INFO | Retried: 0

The one test not passed with the old version is the updated recurrence-test from bug 908745

Comment 3

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

Comment on attachment 831712 [details] [diff] [review]
update_icaljs.diff

r=philipp. I think one more fix was made, feel free to include it too.

Comment 4

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

(In reply to Markus Adrario [:Taraman] from comment #1)
> Created attachment 831712 [details] [diff] [review]
> update_icaljs.diff
> 
> This patch incorporates the latest ical.js from Github.
> 
> The Diff shows the additions from 4 merges on github [1-4]
> 
> This change:
> > -ICAL.foldLength = 74;
> > +ICAL.foldLength = 75;
> I don't understand and took the value from github.
I think there was a certain testcase that failed because the line length was too long. rfc5545 recommends "75 octets", which is obviously not characters, but as there are not more failing tests I think we can keep the github value.



> The next line is:
> > ICAL.debug = false;
> and I left it as it is in comm-central.
Sounds good.

> 
> In line 4447 on comm-Central we have a change from:
> > var dictParts = dict.parts = {};
> to
> > var dictParts = dict.parts = Object.create(null);
> which I could not find in any change history on github.
I guess this was also something that caused the tests to fail, but again, lets keep the github value.

Comment 5

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

Attachment: icaljs-update.diff

Here is an updated version for reference, I needed this for something else too.

Comment 6

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

I've just created a futher PR that we should take: https://github.com/mozilla-comm/ical.js/pull/104

Comment 7

[Markus Adrario [:Taraman]]

Ok, I wait for that to be merged.

Comment 8

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

Merged, go ahead.

Comment 9

[Markus Adrario [:Taraman]]

Attachment: new_fixes.diff

This is a diff between the ical.js from attachment 8376803 [details] [diff] [review] and the new version containing the latest fixes.

please have a look to be sure I have everything.

Comment 10

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

I'm a bit confused. The changes in your latest patch change from
416480ae848ca2db6492a5f72f2dc65d9699ff5d -> 9d3ee67d1ce9e0bb960e73a45e2dd434360cbd8d

attachment 8376803 [details] [diff] [review] (the one you linked in comment 9) changes from:
b2112a5eb6fe2c311ee945150b2a267213c1f18c -> f88963327c65d00c91dbd46b66b758e38c72cf3b

The now obsolete attachment 831712 [details] [diff] [review] changes from:
b2112a5eb6fe2c311ee945150b2a267213c1f18c -> 416480ae848ca2db6492a5f72f2dc65d9699ff5d

So I assume that your new patch applies on top of the now obsolete one? In any case, the extra changes look like the right additional changes, so as long as it applies for you it should be fine.

Comment 11

[Markus Adrario [:Taraman]]

OK, b2112a5eb6fe2c311ee945150b2a267213c1f18c is the revision of the current ical.js in comm-central.

416480ae848ca2db6492a5f72f2dc65d9699ff5d & f88963327c65d00c91dbd46b66b758e38c72cf3b were intermediate revs which must have gotten mixed up in my local queue.
Sorry for the confusion

9d3ee67d1ce9e0bb960e73a45e2dd434360cbd8d is the current revision on github which is the basis for the commit.

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/9cbd527a1075>

-> FIXED

Comment 12

[Martin Schröder [:mschroeder]]

Comment on attachment 8378951 [details] [diff] [review]
new_fixes.diff

I don't think the review is needed anymore.