User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b5) Gecko/20051020 Firefox/1.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051020 Mozilla Sunbird/0.2+ Changing "Tools > Options > Alarms > Show missed alarms" gives JavaScript error when leaving dialog with Ok: Error: subject.function::getBoolPref is not a function Source File: chrome://calendar/content/pref/rootCalendarPref.js Line: 99 Reproducible: Always Steps to Reproduce:

This fixes things for me. As this is my first patch I'm open-minded towards feedback.

Comment on attachment 200279 [details] [diff] [review] Remove unnecessary function call Thanks for the patch! In the future, you should ask for review from someone when submitting a patch. Patches are only checked in once they're properly reviewed. For Sunbird, good reviewers are mvl@exedo.nl or myself. For lightning, dmose@mozilla.org is a good one. Now, on to the patch... It's true that the current call is useless. However, this bug is a symptom of a larger problem in rootCalendarPref.js, namely all of the calls involving subject can fail. For instance, I can produce a similar error by changing the number of weeks in the multiweek view. This time the error is Error: subject.function::getIntPref is not a function Source File: chrome://calendar/content/pref/rootCalendarPref.js Line: 77 A better solution to this bug would fix all of these problems. Hint: You'll need to learn the very basics of the QueryInterface function.

Looks like this fixes the problem for me, not sure this is the way to go though :)

Comment on attachment 200346 [details] [diff] [review] Patch v1 Nope, this isn't it either. This just makes the case be missed by the switch().

2nd guess: Using Components.interfaces.nsIPrefBranch. Not sure if the case for "calendar.alarms.showmissed" is obsolete or not.

Comment on attachment 200352 [details] [diff] [review] Using Components.interfaces.nsIPrefBranch Now we're getting somewhere! :-) This is good, but it can be tightened up a bit. + var pbi = rootPrefNode.QueryInterface(Components.interfaces.nsIPrefBranch2); + rootPrefNode is a bad thing in itself, and I'm actually going to trash it soon, (probably bug 306079). |subject| will actually QI to nsIPrefBranch2 as well. I'd prefer if you used that instead. - newWeeks.setAttribute("value", subject.getIntPref( prefName ) ); + newWeeks.setAttribute("value", pbi.getIntPref( prefName ) ); No need to create another variable, either. Just reuse subject as the result of the QI. That's more of a personal style thing, though. If you really want another variable I won't argue, but call it pb2 instead of pbi (nsIPrefBranchInternal isn't really used anymore) r=jminta with those changes. (Attach another patch with them fixed and I'll check it in.)

Incorporate review comments from comment #6. Rename pbi to pb2, get rid of rootPrefNode, reuse subject.

Comment on attachment 200376 [details] [diff] [review] Using Components.interfaces.nsIPrefBranch2, v2 Thanks for the patch! r=jminta - if( subject.getBoolPref( prefName ) ) { + if (subject.getBoolPref( prefName )) { gDefaultTimezone = subject.getCharPref( prefName ); - if (this.CalendarPreferences.calendarWindow.currentView != null) { In the future, please be careful of random, or pseudo-random whitespace changes. They make cvs blame much more confusing for others to read.

Patch checked in.