Closed Bug 932217 Opened 8 years ago Closed 7 years ago

Update ical.js to latest version

Categories

(Calendar :: ICAL.js Integration, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Taraman, Assigned: Taraman)

References

Details

Attachments

(2 files, 1 obsolete file)

Since there have been some bugfixes recently, we should update the version of ical.js calendar is using.
Attached patch update_icaljs.diff (obsolete) — Splinter Review
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
Assignee: nobody → Mozilla
Status: NEW → ASSIGNED
Attachment #831712 - Flags: review?(philipp)
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 on attachment 831712 [details] [diff] [review]
update_icaljs.diff

r=philipp. I think one more fix was made, feel free to include it too.
Attachment #831712 - Flags: review?(philipp) → review+
(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.
Here is an updated version for reference, I needed this for something else too.
Attachment #831712 - Attachment is obsolete: true
I've just created a futher PR that we should take: https://github.com/mozilla-comm/ical.js/pull/104
Ok, I wait for that to be merged.
Merged, go ahead.
Attached patch new_fixes.diffSplinter Review
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.
Attachment #8378951 - Flags: review?(philipp)
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.
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
OS: Windows XP → All
Resolution: --- → FIXED
Target Milestone: --- → 3.2
Comment on attachment 8378951 [details] [diff] [review]
new_fixes.diff

I don't think the review is needed anymore.
Attachment #8378951 - Flags: review?(philipp)
Attachment #8378951 - Attachment is patch: true
Attachment #8378951 - Attachment mime type: text/x-patch → text/plain
You need to log in before you can comment on or make changes to this bug.