871269 - Using a free-form pref is misleading, change to boolean pref instead

Status: RESOLVED FIXED

(Calendar :: ICAL.js Integration, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Thunderbird/23.0a1 (BuildID: 20130512030916)

Lightning 2.5a1 (BuildID: 20130512042826) from https://ftp.mozilla.org/pub/mozilla.org/calendar/lightning/tinderbox-builds/comm-central-win32/1368358106/

STR:
1. install Lightning
2. set calendar.backend to "jsical"
3. restart Thunderbird

Startup fails. console shows

> Error: NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]
> Source file: chrome://calendar/content/calUtils.js
> Line: 92

> Error: TypeError: Components.classes[cid] is undefined
> Source file: chrome://calendar/content/calUtils.js
> Line: 16

Seems that |Components.classes["@mozilla.org/calendar/datetime-formatter;1"].getService(Components.interfaces.calIDateTimeFormatter)| fails in case of using jsical. Works in case of using libical.

Comment 1

[Matthew Mecca [:mmecca]]

(In reply to Stefan Sitter from comment #0)

> 2. set calendar.backend to "jsical"

Should be "icaljs"

Comment 2

[Stefan Sitter [:ssitter]]

Maybe I got confused because the landed bug is called jsical. I think the preference should be more user friendly. A preference that takes a free string is very error prone if not handled correctly, as it seems to be done in this case.

Why not just use a simple boolean preference like calendar.backend.icaljs.enabled?

Comment 3

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

Attachment: Fix - v1

If we want to change this we should do it now, I'd like to blog about this tomorrow. I'm fine with making it a boolean pref since ideally it will be temporary anyway.

This patch untested, sorry, but if you could take a look and agree, please push. Maybe we can get this in before the nightly.

Comment 4

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

Pushed to comm-central changeset 1f435c4e5b43

Comment 5

[Stefan Sitter [:ssitter]]

Now that I had time for testing I see that it is not working. Fails with
> ### Error loading backend: ReferenceError: backend is not defined

You left a debug statement in the code that still references backend
> http://mxr.mozilla.org/comm-central/source/calendar/base/backend/calBackendLoader.js#62
> dump("[calBackendLoader] Using " + backend + " backend at " + file.path + "\n");

Comment 6

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

Attachment: Fix - v2

Gah, sorry. This should take care. Again untested, my comm-central is currently broken due to m-c changes.

Let me know when I should push, or just go ahead and push yourself.

Thanks for the testing and reviews!

Comment 7

[Stefan Sitter [:ssitter]]

Comment on attachment 748944 [details] [diff] [review]
Fix - v2

We need to port this back to comm-aurora for Lightning 2.5 too.

Now I can use icaljs but performance seems very slow compared to libical rendering calendar view almost unusable.

Comment 8

[Stefan Sitter [:ssitter]]

Comment on attachment 748944 [details] [diff] [review]
Fix - v2

>+        let backend = "libical"

missing ";"

Comment 9

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

(In reply to Stefan Sitter from comment #7)
> Comment on attachment 748944 [details] [diff] [review]
> Fix - v2
> 
> We need to port this back to comm-aurora for Lightning 2.5 too.
Yes, I will push to both branches in a moment.
 
> Now I can use icaljs but performance seems very slow compared to libical
> rendering calendar view almost unusable.
Matthew mentioned this via email too. I'm not yet sure what the problem is, I had the feeling it was fast when I was testing. We're going to have to optimize this and find out where the issues are.

One possible lead is that the timezone components are loaded lazily for libical and parsed directly with ical.js.

Comment 10

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

Pushed to comm-central changeset f1cf1ac78c06

Comment 11

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

Aurora is still marked closed for merges, will push as soon as its open.

Comment 12

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

Backported to releases/comm-aurora changeset 89eea795de11