Closed Bug 354574 Opened 14 years ago Closed 13 years ago

Centralize definition and getters for PRODID and VERSION

Categories

(Calendar :: Provider: ICS/WebDAV, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mattwillis, Assigned: richwklein)

References

Details

Attachments

(1 file, 2 obsolete files)

Spinoff from bug 328576:

>+
>+        const icssvc = Cc["@mozilla.org/calendar/ics-service;1"].
>+                       getService(Components.interfaces.calIICSService);
>+        var modifiedItem = icssvc.createIcalComponent("VCALENDAR");
>+        modifiedItem.prodid = "-//Mozilla Calendar//NONSGML Sunbird//EN";
>+        modifiedItem.version = "2.0";
>
>Please file a spinoff bug about centralizing the prodid and version somewhere,
>probably in calIICSService.
Not going to make the 0.5 train.
Target Milestone: Sunbird 0.5 → ---
Attached patch rev0 - use same PRODID (obsolete) — Splinter Review
Currently Lightning sends out invitations containing the PRODID "-//Mozilla Calendar//NONSGML Sunbird//EN". This patch takes care that we always use the same PRODID "-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN" that is used in other places.

The next step would be to centralize this.
Assignee: nobody → ssitter
Status: NEW → ASSIGNED
Attachment #281476 - Flags: review?(daniel.boelzle)
Comment on attachment 281476 [details] [diff] [review]
rev0 - use same PRODID

You are aware that this totally does _not_ fix this bug, but in fact shows the need for this bug to be solved in a proper way?
(In reply to comment #3)
Yes, see Comment #2. I tried several ways to centralize it but none of the solutions worked out. Therefore my patch takes care that we use the same PRODID at least instead of different ones. If you have a working solution feel free to overwrite my patch.
Attached patch Patch v1 (obsolete) — Splinter Review
This patch adds a function in calUtils.js that sets the prodid and version for an icalcomponent passed into it.  This seems to work in the limited testing I've done for it.  Let me know if this is the correct direction.
Attachment #285031 - Flags: review?(mvl)
Comment on attachment 281476 [details] [diff] [review]
rev0 - use same PRODID

we should go with the second patch and centralize the literal.
Attachment #281476 - Flags: review?(daniel.boelzle) → review-
Comment on attachment 285031 [details] [diff] [review]
Patch v1

stealing review from mvl...

>+/**
>+ * This is a centralized function for setting the prodid and version on an
>+ * ical components.  This should be used whenever you need to set the prodid
>+ * and version on an calicalcomponent object.
>+ *
>+ * PARAMETERS
>+ *      aIcalComponent - The ical component to set the prodid and version on.
>+ */
>+function setProdidVersion(aIcalComponent) {
please prefix with "cal"

>+  // return early not an ical component
>+  if (!aIcalComponent instanceof Components.interfaces.calIIcalComponent)
>+      return;
please throw Components.results.NS_ERROR_INVALID_ARG

r=dbo with the above resolved (would be nice if you could provide an updated patch).
Attachment #285031 - Flags: review?(mvl) → review+
Attachment #281476 - Attachment is obsolete: true
Assignee: ssitter → richwklein
Status: ASSIGNED → NEW
Version: Trunk → unspecified
Daniel, Do you want the function or the parameter prefixed with "cal"?  I'll update the patch with those changes once if you could clarify that for me.
The function, e.g. function calSetProdIdVersion(aIcalComponent).
Attached patch Patch v2Splinter Review
Here is an update patch with the changes you requested.
Attachment #285132 - Flags: review+
Attachment #285031 - Attachment is obsolete: true
Attachment #285031 - Flags: review+
Status: NEW → ASSIGNED
Keywords: checkin-needed
Whiteboard: [checkin-needed after 0.7]
Checked in on HEAD and MOZILLA_1_8_BRANCH; thanks for the patch.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [checkin-needed after 0.7]
Target Milestone: --- → 0.8
Depends on: 404763
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.