Remove e4x usage in the gdata provider (cpg fallout)

RESOLVED FIXED in 1.7

Status

Calendar
Provider: GData
--
blocker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

Lightning 1.7
Dependency tree / graph

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
*sadfaces* CPG has broken e4x for the gdata provider, so I need to solve things differently for the Lightning 1.7 release.
(Assignee)

Comment 1

5 years ago
Created attachment 626047 [details] [diff] [review]
Fix - v1

This removes e4x usage. I hope I didn't miss anything and that everything works!
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #626047 - Flags: review?(matthew.mecca)

Comment 2

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

Comment 3

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

Updated

5 years ago
Version: Trunk → Lightning 1.7

Comment 4

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

Comment 5

5 years ago
Don't worry, 1.7 is at least 12 weeks down the road :-)

Comment 6

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

Comment 7

5 years ago
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

5 years ago
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 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.
Attachment #626047 - Flags: review?(matthew.mecca) → review+
(Assignee)

Comment 10

5 years ago
I did a few more style fixes and also fixed the above comments. Pushing is imminent.
(Assignee)

Comment 11

5 years ago
Pushed to comm-central changeset 2769eaf6b5bf
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.7

Comment 12

5 years ago
Looks good from here, Philipp (I applied the diff from the changeset). Congrats!

Updated

5 years ago
Duplicate of this bug: 786457

Updated

5 years ago
Duplicate of this bug: 786578
You need to log in before you can comment on or make changes to this bug.