Filed following discussion in Bug 752281. Lightning is broken by CPG because it uses E4X.
This is the original error report from Bug 752281: -------------------------------------- Timestamp: 05/06/2012 12:23:59 AM Error: Assert failed: [Exception... "'TypeError: can't wrap XML objects' when calling method: [calICalDavCalendar::refresh]" nsresult: "0x8057001c (NS_ERROR_XPC_JS_THREW_JS_OBJECT)" location: "JS frame :: file:///Data/Profiles/Thunderbird/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calCompositeCalendar.js :: cCC_refresh :: line 378" data: no] 2: [null:0] null 3: [file:///Data/Profiles/Thunderbird/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/components/calCompositeCalendar.js:381] cCC_refresh 4: [null:0] null 5: [chrome://calendar/content/calendar-common-sets.js:402] cC_doCommand 6: [chrome://global/content/globalOverlay.js:98] goDoCommand 7: [chrome://messenger/content/messenger.xul:1] oncommand 8: [null:0] null Source File: resource://calendar/modules/calUtils.jsm -> file:///Data/Profiles/Thunderbird/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calUtils.js Line: 1105 --------------------------------------
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Bug 650353 breaks Lightning with [TypeError: can't wrap XML objects] → Bug 650353 breaks Lightning with [TypeError: can't wrap XML objects] due to e4x usage
Version: Trunk → Lightning 1.7
Oh don't tell me you removed e4x support, or at least effectively broke it with CPG? I was told there shall be an easy replacement to e4x before it is to be removed? And by easy I don't mean document.createElement().
Severity: major → blocker
Component: General → Provider: CalDAV
QA Contact: general → caldav-provider
Usually I shouldn't do things like this while on vacation, but this bug personally aggravates me ;-) This patch works around by serializing the e4x directly in the function it is created in, not passing e4x to other compartments. I still need an answer about a replacement for e4x. Bluefang, maybe you could CC some people and/or tell me what the promised replacement is?
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #623499 - Flags: review?(matthew.mecca)
Forgot to qrefresh, now with gdata changes too.
I'm still getting a DAV_NOT_DAV error here http://mxr.mozilla.org/comm-central/source/calendar/providers/caldav/calDavCalendar.js#1619 It looks like cal.safeNewXML is still affected.
I don't think that cal.safeNewXML is the only problem. Looks to me like e4x is broken in js-land, so all our multistatus..D::whatever parsing is broken as well. Might be time for xpath or similar.
I was able to get a Google CAlDAV calendar working again with Workaround v1 + this fix, but a similar fix would be needed for the GData and probably ICS providers - is there a safe place we could move the safeNewXML function to that could be shared between providers without crossing compartments?
(In reply to Philipp Kewisch [:Fallen] (away until May 28th) from comment #3) > I still need an answer about a replacement for e4x. Bluefang, maybe you > could CC some people and/or tell me what the promised replacement is? Sorry, I'm just an end-user who files bugs. All I know about the issue is what's said in Bug 752281. E4X is broken because it's not allowed for cross-compartment wrappers. It looks like E4X has been on the road to depreciation and removal for at least 1/2 a year. Given that only Mozilla and Adobe support it, and the complexity and security considerations, that doesn't sound like a bad thing to me. Here's some relevant stuff I've found: Bug 485791 - Self-host the E4X runtime in JS using proxies Bug 613142 - "can't wrap XML objects" Bug 695577 - E4X syntax should not be accepted in ES5 strict mode https://groups.google.com/d/topic/mozilla.dev.tech.js-engine/4iMtEASXxL8/discussion https://developer.mozilla.org/en/E4X It looks like JXON is the recommended alternative: https://developer.mozilla.org/en/JXON
That sounds reasonable. IIUC e4x isn't being dropped completely right now, and should continue to work as long as we can keep it contained to individual compartments. I think the safest bet is to use the workarounds for now to get everything working again, and buy some time to put the replacement in place and make sure it's tested thoroughly.
BTW: I tried some mad hacks using Components.utils.getGlobalForObject() and friends, but cross compartment wrappers and the fact that func.caller is broken across jsm boundaries (bug 586453) seem to be bashing this. Before I spend more time trying to find a hack, I'll rather spend time getting rid of e4x. If you want a quick fix feel free to fold the patches and push them, r=me.
This patch takes a first stab at getting rid of e4x. I've tested this with a Zimbra and SOGo server and it works (even freebusy), but I couldn't test all queries (i.e principal-property-search). Maybe you have a different caldav server handy for testing? Please stop me from writing a caldav fakeserver just to fix this bug ;-) If this works out fine, I still need to find a solution for the gdata provider, as it uses e4x very much more extensively. I guess it will be similar, or I would rewrite to use gdata's JSON output (yuck, that almost means rewrite to use the latest gdata api). Maybe I'll just move safeNewXML to the gdata compartment and take care later. Matthew, what do you think of this patch?
Alas, I need to look though calDavRequestHandlers too. It mostly uses SAX, but it does use e4x to put together some of the queries.
(In reply to Philipp Kewisch [:Fallen] (away until May 28th) from comment #12) This looks good. > This patch takes a first stab at getting rid of e4x. I've tested this with a > Zimbra and SOGo server and it works (even freebusy), but I couldn't test all > queries (i.e principal-property-search). Maybe you have a different caldav > server handy for testing? Please stop me from writing a caldav fakeserver > just to fix this bug ;-) Seems to be working well with Google Calendar via CalDAV, unfortunately I don't have another CalDAV server to test with at the moment. > If this works out fine, I still need to find a solution for the gdata > provider, as it uses e4x very much more extensively. I guess it will be > similar, or I would rewrite to use gdata's JSON output (yuck, that almost > means rewrite to use the latest gdata api). Maybe I'll just move safeNewXML > to the gdata compartment and take care later. I think the safeNewXML workaround should work for now. We're going to have to move to the new version of the gdata api at some point anyway, maybe we can get task support as a bonus if we look into that soon.
Attachment #624680 - Flags: feedback?(matthew.mecca) → feedback+
I think this is ready for review. I'd appreciate if you could do some thorough testing to make sure this doesn't break anything :-)
Argh, it seems something is not working for me, getting empty calendars with Zimbra.
Oh, that was easy. I accidentally put the <D:href> elements into the <D:prop>.
Comment on attachment 625453 [details] [diff] [review] Fix - v3 Review of attachment 625453 [details] [diff] [review]: ----------------------------------------------------------------- CalDAV via Google Calendar and WebDAV seem to be working fine. I'm still getting the "can't wrap XML objects" error with the GData Provider, but if you want to take care of that in a followup bug I'm fine with that. r=mmecca ::: calendar/base/modules/Makefile.in @@ +55,4 @@ > calPrintUtils.jsm \ > calProviderUtils.jsm \ > calUtils.jsm \ > + calXMLUtils.jsm \ Looks like a tab snuck in here.
Attachment #625453 - Flags: review?(matthew.mecca) → review+
(In reply to Matthew Mecca [:mmecca] from comment #18) > Comment on attachment 625453 [details] [diff] [review] > Fix - v3 > > Review of attachment 625453 [details] [diff] [review]: > ----------------------------------------------------------------- > > CalDAV via Google Calendar and WebDAV seem to be working fine. I'm still > getting the "can't wrap XML objects" error with the GData Provider, but if > you want to take care of that in a followup bug I'm fine with that. r=mmecca Oh boy, this sucks: each script loaded via subscript loader has its own global (which I guess is expected), but that means I can't use an e4x object created in calGoogleUtils.js anywhere in calGoogleCalendar.js. This means I will have to change everything afterall :-( Off to a separate bug, reverting the gdata changes in the above patch and leaving it broken. > > ::: calendar/base/modules/Makefile.in > @@ +55,4 @@ > > calPrintUtils.jsm \ > > calProviderUtils.jsm \ > > calUtils.jsm \ > > + calXMLUtils.jsm \ > > Looks like a tab snuck in here. Good catch, fixed.
Pushed to comm-central changeset a182ec1d232f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.7
You need to log in before you can comment on or make changes to this bug.