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

RESOLVED FIXED in 2.5

Status

Calendar
ICAL.js Integration
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Stefan Sitter, Assigned: Fallen)

Tracking

Lightning 2.5
x86_64
Windows 7

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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.
(In reply to Stefan Sitter from comment #0)

> 2. set calendar.backend to "jsical"

Should be "icaljs"
(Reporter)

Comment 2

5 years ago
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?
(Assignee)

Comment 3

5 years ago
Created attachment 748583 [details] [diff] [review]
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.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #748583 - Flags: review?(ssitter)
(Assignee)

Updated

5 years ago
Summary: [jsical] startup error, nothing works [Error: NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]] → [jsical] Using a free-form pref is misleading, change to boolean pref instead
(Reporter)

Updated

5 years ago
Attachment #748583 - Flags: review?(ssitter) → review+
(Assignee)

Updated

5 years ago
Component: Internal Components → ICAL.js Integration
Summary: [jsical] Using a free-form pref is misleading, change to boolean pref instead → Using a free-form pref is misleading, change to boolean pref instead
(Assignee)

Comment 4

5 years ago
Pushed to comm-central changeset 1f435c4e5b43
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.5
(Reporter)

Comment 5

5 years ago
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");
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

5 years ago
Created attachment 748944 [details] [diff] [review]
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!
Attachment #748944 - Flags: review?(ssitter)
(Reporter)

Comment 7

5 years ago
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.
Attachment #748944 - Flags: review?(ssitter)
Attachment #748944 - Flags: review+
Attachment #748944 - Flags: approval-calendar-aurora+
(Reporter)

Comment 8

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

>+        let backend = "libical"

missing ";"
(Assignee)

Comment 9

5 years ago
(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.
(Assignee)

Comment 10

5 years ago
Pushed to comm-central changeset f1cf1ac78c06
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: 2.5 → 2.6
(Assignee)

Comment 11

5 years ago
Aurora is still marked closed for merges, will push as soon as its open.
(Assignee)

Comment 12

5 years ago
Backported to releases/comm-aurora changeset 89eea795de11
Target Milestone: 2.6 → 2.5
You need to log in before you can comment on or make changes to this bug.