Closed
Bug 404085
Opened 17 years ago
Closed 17 years ago
Consolidate provider methods into a default calendar implementation
Categories
(Calendar :: Internal Components, defect)
Calendar
Internal Components
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: Fallen, Assigned: Fallen)
References
Details
(Whiteboard: [gdata-0.4])
Attachments
(1 file, 2 obsolete files)
123.30 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
We have a lot of stuff that is done the same in each providers. Using js inheritance, we can consolidate all that into a default calendar implementation. This patch takes care.
Attachment #289034 -
Flags: review?(daniel.boelzle)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [gdata-cvs]
Target Milestone: --- → 0.8
Assignee | ||
Comment 1•17 years ago
|
||
Some small fixes to first patch
Attachment #289034 -
Attachment is obsolete: true
Attachment #289181 -
Flags: review?(daniel.boelzle)
Attachment #289034 -
Flags: review?(daniel.boelzle)
Comment 2•17 years ago
|
||
Comment on attachment 289181 [details] [diff] [review] Create Default Calendar Implementation >+++ calendar/providers/default/calDefaultCalendar.js 2007-11-18 01:12:23.471211200 +0100 I'd prefer calCalendarBase or calProviderBase (or similar) expressing that it is meant for inheritance only. Add an ASSERT(false, "just inherit, call!") or throw in calCalendarBase, too. >+ QueryInterface: function cDC_QueryInterface() { >+ if (!aIID.equals(Components.interfaces.nsISupports) && >+ !aIID.equals(Components.interfaces.calICalendar)) { >+ throw Components.results.NS_ERROR_NO_INTERFACE; >+ } use ensureIID for brevity. >+const kStorageServiceContractID = "@mozilla.org/storage/service;1"; >+const kStorageServiceIID = Components.interfaces.mozIStorageService; >+ >+const kCalICalendar = Components.interfaces.calICalendar; >+ >+const kCalAttendeeContractID = "@mozilla.org/calendar/attendee;1"; >+const kCalIAttendee = Components.interfaces.calIAttendee; >+var CalAttendee; >+ >+const kCalRecurrenceInfoContractID = "@mozilla.org/calendar/recurrence-info;1"; >+const kCalIRecurrenceInfo = Components.interfaces.calIRecurrenceInfo; >+var CalRecurrenceInfo; >+ >+const kCalRecurrenceRuleContractID = "@mozilla.org/calendar/recurrence-rule;1"; >+const kCalIRecurrenceRule = Components.interfaces.calIRecurrenceRule; >+var CalRecurrenceRule; >+ >+const kCalRecurrenceDateSetContractID = "@mozilla.org/calendar/recurrence-date-set;1"; >+const kCalIRecurrenceDateSet = Components.interfaces.calIRecurrenceDateSet; >+var CalRecurrenceDateSet; >+ >+const kCalRecurrenceDateContractID = "@mozilla.org/calendar/recurrence-date;1"; >+const kCalIRecurrenceDate = Components.interfaces.calIRecurrenceDate; >+var CalRecurrenceDate; >+ >+const kMozStorageStatementWrapperContractID = "@mozilla.org/storage/statement-wrapper;1"; >+const kMozStorageStatementWrapperIID = Components.interfaces.mozIStorageStatementWrapper; >+var MozStorageStatementWrapper; >+ >+if (!kMozStorageStatementWrapperIID) { >+ dump("*** mozStorage not available, calendar/storage provider will not function\n"); >+} >+ >+function initCalStorageCalendarComponent() { >+ CalAttendee = new Components.Constructor(kCalAttendeeContractID, kCalIAttendee); >+ CalRecurrenceInfo = new Components.Constructor(kCalRecurrenceInfoContractID, kCalIRecurrenceInfo); >+ CalRecurrenceRule = new Components.Constructor(kCalRecurrenceRuleContractID, kCalIRecurrenceRule); >+ CalRecurrenceDateSet = new Components.Constructor(kCalRecurrenceDateSetContractID, kCalIRecurrenceDateSet); >+ CalRecurrenceDate = new Components.Constructor(kCalRecurrenceDateContractID, kCalIRecurrenceDate); >+ MozStorageStatementWrapper = new Components.Constructor(kMozStorageStatementWrapperContractID, kMozStorageStatementWrapperIID); >+} why did you move the above code from calStorageCalendar? >+ try { >+ var fileurl = iosvc.newFileURI(f); >+ loader.loadSubScript(fileurl.spec, null); >+ } catch (e) { >+ dump("Error while loading " + fileurl.spec + "\n"); >+ throw e; >+ } please repair all the module code to use Components.util.reportError instead of dump. >+++ calendar/providers/caldav/calDavCalendar.js 17 Nov 2007 23:51:29 -0000 > function calDavCalendar() { >- this.wrappedJSObject = this; >- this.mObservers = new calListenerBag(Components.interfaces.calIObserver); > this.unmappedProperties = []; > this.mPendingStartupRequests = []; > this.mUriParams = null; > this.mEtagCache = []; > this.mDisabled = false; >+ this.initDefaultCalendar(); > } call base ctors before anything selse. > QueryInterface: function (aIID) { > if (!aIID.equals(Components.interfaces.nsISupports) && > !aIID.equals(Components.interfaces.calICalendarProvider) && > !aIID.equals(Components.interfaces.calICalendar) && > !aIID.equals(Components.interfaces.nsIInterfaceRequestor)) { > throw Components.results.NS_ERROR_NO_INTERFACE; > } I'd prefer you revise and shorten QueryInterface implementations, too, e.g. adding // yet untested! function queryInterface(that, aList, aIID) { function checkIID(iid) { return iid.equals(aIID); } if (aList.some(checkIID)) { return that; } var base = that.__proto__.__proto__; if (base && base.QueryInterface) { return base.QueryInterface.call(that, aIID); } throw Components.results.NS_ERROR_NO_INTERFACE; } to calUtils.js > webSvc.report(calendarDirResource, queryDoc, true, reportListener, > this, null); >- return; >+ return; why leave this return statement? > function calMemoryCalendar() { >- this.wrappedJSObject = this; > this.calendarToReturn = this, > this.initMemoryCalendar(); >+ this.initDefaultCalendar(); > } base ctor before anything else r=dbo with that fixed/explained/resolved
Attachment #289181 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 3•17 years ago
|
||
comments as discussed, will elaborate later. Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 4•17 years ago
|
||
This broke all calendar-related builds on trunk: gmake[8]: *** No rule to make target `calStorageCalendarModule.js'. Stop. gmake[8]: Leaving directory `/mnt/mozilla/build/seamonkey-dbg/calendar/providers/storage'
Assignee | ||
Comment 5•17 years ago
|
||
(In reply to comment #2) > why did you move the above code from calStorageCalendar? Moved back and code changed as discussed > I'd prefer you revise and shorten QueryInterface implementations, too, e.g. > adding [...] Added doQueryInterface to calUtils.js > > webSvc.report(calendarDirResource, queryDoc, true, reportListener, > > this, null); > >- return; > >+ return; > why leave this return statement? This was just a result of my autoclean script, I didn't really touch this segment of the code, and since some local problems made creating the second patch pretty annoying I left this in for now. Everything else taken care of.
Attachment #289181 -
Attachment is obsolete: true
Attachment #289526 -
Flags: review+
Assignee | ||
Comment 6•17 years ago
|
||
Missing file also checked in.
Assignee | ||
Updated•16 years ago
|
Whiteboard: [gdata-cvs] → [gdata-0.4]
You need to log in
before you can comment on or make changes to this bug.
Description
•