initial patch for this, just adds central control of the start/end time. Need to do some pref lookups in the view constructor and maybe attach a pref observer for changes.

Comment on attachment 187443 [details] [diff] [review] start/end time setting, no pref yet r=shaver

First part of this checked in; leaving bug open until I do the pref hookup in a second patch.

Sunbird supports: calendar.event.defaultstarthour calendar.event.defaultendhour with a value of 0-23. We have some stuff in the lightning prefs dialog for these, although its kind of ugly. Use these prefs and default to whatever we do currently.

After the last checkin, all that was left to do was simply watch the prefs and update the multiday view if they changed. This patch does that. I have a slight concern the ltnFinish() isn't getting called, but in principle, it should be.

Comment on attachment 197338 [details] [diff] [review] pref observer Looks good; just a few minor nits. In general, prefer coding an explicit |return| at the end of functions rather than falling off the end. It makes it clear to the reader that there was intent to return and not an inadvertant error. >Index: mozilla/calendar/lightning/content/messenger-overlay-sidebar.js >=================================================================== >RCS file: /cvsroot/mozilla/calendar/lightning/content/messenger-overlay-sidebar.js,v >retrieving revision 1.22 >diff -p -U8 -r1.22 messenger-overlay-sidebar.js >--- mozilla/calendar/lightning/content/messenger-overlay-sidebar.js 23 Sep 2005 01:09:47 -0000 1.22 >+++ mozilla/calendar/lightning/content/messenger-overlay-sidebar.js 25 Sep 2005 15:57:50 -0000 >@@ -65,16 +65,30 @@ function ltnOnLoad(event) > > document.getElementById("ltnMinimonth").value = today; > > gMiniMonthLoading = false; > > // nuke the onload, or we get called every time there's > // any load that occurs > document.removeEventListener("load", ltnOnLoad, true); >+ >+ // Start observing preferences >+ var prefService = Components.classes["@mozilla.org/preferences-service;1"] >+ .getService(Components.interfaces.nsIPrefService); >+ var rootPrefBranch = prefService.getBranch(""); >+ ltnPrefObserver.rootPrefBranch = rootPrefBranch; >+ var pbi = rootPrefBranch.QueryInterface( >+ Components.interfaces.nsIPrefBranch2); >+ pbi.addObserver("calendar.", ltnPrefObserver, false); Maybe just call this "pb" or "pb2" since the interface is no longer called nsIPrefBranchInternal? >+ ltnPrefObserver.observe(null, null, ""); >+ >+ // Add an unload function to the window so we don't leak the pref observer >+ var win = document.getElementById("messengerWindow"); >+ win.setAttribute("onunload", "ltnFinish(); "+win.getAttribute("onunload")); Does this really want to be |win.addEventListener()|?

Version 2 fixes the comments from the previous review. It also explicitly sets the start/end hours each startup, since the views have no way to persist these things on their own.

Comment on attachment 197640 [details] [diff] [review] pref observer v2 r=dmose. Thanks for the patch!

Comment on attachment 197640 [details] [diff] [review] pref observer v2 patch checked in, leaving bug open to sort out the start/end of week issues

This patch wires in a preference for the day that the week should start on by default. Setting it currently won't have any effect, but with the decorated week-view it works. (This bug doesn't say anything about getting a default starting day of the week for the month view. I'm guessing that should be handled separately?) This patch also creates a default preferences file (like any good extension should have) that gets shipped in the lightning.xpi and is automatically placed in the right location during the xpinstall process. I can break this part off into a separate bug if you prefer.

Comment on attachment 205443 [details] [diff] [review] pref for start of week Two requests: make the menulist you're adding be accessible, and post a screenshot.

Screenshot of the relevant tab of the lightning preferences with this patch applied. (Note that the code for this is taken directly from Sunbird, so testing is easily available there.)

Same as before, but with align="center" on the row, so that the label lines up nicely. As I mentioned in IRC, xul:menuitem is already accessible by default.

Comment on attachment 206418 [details] [diff] [review] pref for start of week v2 r=dmose

patch checked in. Closing bug since bug 319701 ought to finish this up.