Last Comment Bug 757332 - Remove e4x usage in the gdata provider (cpg fallout)
: Remove e4x usage in the gdata provider (cpg fallout)
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Provider: GData (show other bugs)
: Lightning 1.7
: All All
: -- blocker (vote)
: 1.7
Assigned To: Philipp Kewisch [:Fallen]
:
:
Mentors:
: 786457 786578 (view as bug list)
Depends on: cpg
Blocks: 754164
  Show dependency treegraph
 
Reported: 2012-05-21 22:56 PDT by Philipp Kewisch [:Fallen]
Modified: 2012-08-29 01:43 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix - v1 (47.51 KB, patch)
2012-05-22 09:08 PDT, Philipp Kewisch [:Fallen]
matthew.mecca: review+
Details | Diff | Splinter Review

Description Philipp Kewisch [:Fallen] 2012-05-21 22:56:34 PDT
*sadfaces* CPG has broken e4x for the gdata provider, so I need to solve things differently for the Lightning 1.7 release.
Comment 1 Philipp Kewisch [:Fallen] 2012-05-22 09:08:24 PDT
Created attachment 626047 [details] [diff] [review]
Fix - v1

This removes e4x usage. I hope I didn't miss anything and that everything works!
Comment 2 Lewis Rosenthal 2012-05-22 09:53:30 PDT
Phillip, can this be applied to versions of Provider running under earlier Lightning releases, or will I need to wait to test until I have a working Lightning 1.7 build?
Comment 3 Philipp Kewisch [:Fallen] 2012-05-22 10:01:11 PDT
The patch will work with earlier releases too, as the techniques used are standard DOM and XPath, but its only broken in Lightning 1.7 and the matching Thunderbird where the CPG patch has landed.
Comment 4 Lewis Rosenthal 2012-05-22 10:12:35 PDT
Thanks, Philipp (apologies for the typo last time!). I'll patch my current installation to see if anything odd happens. I don't do the Lightning builds for OS/2, so I'm not sure when Dave will be putting a 1.7 together. It's good to know that this shouldn't break backward compatibility.
Comment 5 Philipp Kewisch [:Fallen] 2012-05-22 18:16:40 PDT
Don't worry, 1.7 is at least 12 weeks down the road :-)
Comment 6 Lewis Rosenthal 2012-05-22 19:56:37 PDT
Well, we weren't so lucky insofar as backward compatibility is concerned. I noticed it in the diff, but didn't look much further.

The diff references:

+Components.utils.import("resource://calendar/modules/calXMLUtils.jsm");

which doesn't exist under Lightning 1.2.3 (at least, not on my system).

So, upon applying the diff and restarting, I get:

Tue May 22 2012 22:28:35
Error: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://calendar/modules/calUtils.jsm -> file:///J:/OS2WEB/Mozilla/SeaMonkey/Profiles/4h0tldty.default/extensions/%7Ba62ef8ec-5fdc-40c2-873c-223b8a6925cc%7D/js/calGoogleUtils.js :: <TOP_LEVEL> :: line 40"  data: no] (file:///J:/OS2WEB/Mozilla/SeaMonkey/Profiles/4h0tldty.default/extensions/%7Ba62ef8ec-5fdc-40c2-873c-223b8a6925cc%7D/js/calGoogleUtils.js)
Source file: resource://calendar/modules/calUtils.jsm
Line: 86

And it's not just an easy matter of pulling calXMLUtils.jsm and/or the updated calUtils.jsm from cvs and dropping them in the Lightning /modules directory, either.

So, it looks like this isn't as backward-compatible as I'd hoped, but no matter. I'll wait until I'm in a better position to test. ;-)
Comment 7 Philipp Kewisch [:Fallen] 2012-05-22 20:12:45 PDT
Oh right, forgot that one. You can get the file here:

http://mxr.mozilla.org/comm-central/source/calendar/base/modules/calXMLUtils.jsm?raw=1

and then just drop it into this path:

J:/OS2WEB/Mozilla/SeaMonkey/Profiles/4h0tldty.default/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/modules/
Comment 8 Lewis Rosenthal 2012-05-22 21:36:39 PDT
Isn't that odd... I did that before (see my comment 6), but I still got an error in the console (referencing a missing ";" in calUtils.jsm, but running the patch again this time and restarting (with the calXLMUtils.jsm in place), it all worked. I guess my initial patch must have not quite gone through.

Anyway, it's up and running now, so I'll play wiht it for a couple of days and will let you know if I find anything which is not working right.

Thanks!
Comment 9 Matthew Mecca [:mmecca] 2012-05-23 06:09:19 PDT
Comment on attachment 626047 [details] [diff] [review]
Fix - v1

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

Looks good. r=mmecca

::: calendar/providers/gdata/components/calGoogleCalendar.js
@@ +982,5 @@
>              this.ensureSession();
>  
>              // Get the item entry by id.
> +            let itemXPath = 'atom:feed/atom:entry[substring-before(atom:id/text(), "' + aOperation.itemId + '")!="" or gCal:uid/@value="' + aOperation.itemId + '"]'
> +            var itemEntry = gdataXPathFirst(xml, itemXPath);

Missing a ';'. var could be changed to let also.

::: calendar/providers/gdata/components/calGoogleUtils.js
@@ +368,5 @@
>   */
>  function ItemToXMLEntry(aItem, aCalendar, aAuthorEmail, aAuthorName) {
>  
> +
> +    let selfIsOrganizer = (!aItem.organizer ||

extra blank line.
Comment 10 Philipp Kewisch [:Fallen] 2012-05-23 18:32:00 PDT
I did a few more style fixes and also fixed the above comments. Pushing is imminent.
Comment 11 Philipp Kewisch [:Fallen] 2012-05-23 18:40:47 PDT
Pushed to comm-central changeset 2769eaf6b5bf
Comment 12 Lewis Rosenthal 2012-05-24 19:01:56 PDT
Looks good from here, Philipp (I applied the diff from the changeset). Congrats!
Comment 13 Stefan Sitter 2012-08-28 22:31:52 PDT
*** Bug 786457 has been marked as a duplicate of this bug. ***
Comment 14 Stefan Sitter 2012-08-29 01:43:12 PDT
*** Bug 786578 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.