1448717 - caldav calendars fail with xmlHeader not defined error

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Cyrus Harmon]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180322223039

Steps to reproduce:

Set up a CalDAV calendar, attempt to read the calendar items.


Actual results:

no calendar entries load. The developer toolbox console says that "xmlHeader" is not defined.

ReferenceError: xmlHeader is not defined[Learn More]
calDavRequestHandlers.js:331:1
NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "xmlHeader is not defined" {file: "resource://calendar/modules/calUtils.jsm -> file:///usr/local/lib/thunderbird/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calDavRequestHandlers.js" line: 331}]'[JavaScript Error: "xmlHeader is not defined" {file: "resource://calendar/modules/calUtils.jsm -> file:///usr/local/lib/thunderbird/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calDavRequestHandlers.js" line: 331}]' when calling method: [calIOperationListener::onOperationComplete]


Expected results:

Calendar events should have loaded.

Comment 1

[[:MakeMyDay]]

There seems to have been a change in dealing with the scope in loaded subscripts - up to now the code has been falling back to the loading script if an undefined var occured. We could easily add a definition for xmlHeader (and MIME_TEXT_XML as well) to calDavRequestHandler or move all the definitions from calDavCalendar there, but probably we should have a look at the loading mechanism, since this may have further yet uncovered impact. Philipp, what do you think?

Comment 2

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

Attachment: Wip caldav refactoring

Not that I have cleaned this up, but I did some refactoring that I will need for the autodiscovery patch, and that should fix the xml header thing.

Comment 3

[[:MakeMyDay]]

Philipp, what do we do with this with regard to TB63? Your patch is a complete refactoring and still WIP. Would it make sense to implement a resolution just for this issue in the first place and do the rest in a follow-up bug?

Comment 4

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

Yes, we can do that. The patch above is pretty elaborate and hence has its own regression risks.

Comment 5

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

Attachment: Fix - v1

Comment 6

[[:MakeMyDay]]

Comment on attachment 9025329 [details] [diff] [review]
Fix - v1

Review of attachment 9025329 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, looks good.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0a1674c405a5
Define missing caldav variables after loader changes. r=MakeMyDay

Comment 8

[Jorg K (CEST = GMT+2)]

Beta (TB 64 / Cal 6.6 beta 3):
https://hg.mozilla.org/releases/comm-beta/rev/0232450e2c9b