Closed Bug 97220 Opened 19 years ago Closed 19 years ago

Add Run Once function on startup

Categories

(SeaMonkey :: Installer, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: kmurray1115, Assigned: samir_bugzilla)

Details

(Whiteboard: Have patch, r, sr, a)

Attachments

(3 files)

Tracking bug. Add ability to run a unique startup page after version changes.
Provides ability to provide version-specific info on startup for upgraders.
jag, please r.
blake, please sr.

(This was bugscape 7230.)
Status: NEW → ASSIGNED
OS: Windows 2000 → All
Priority: -- → P2
Hardware: PC → All
Whiteboard: Have patch; need r, sr, a
Target Milestone: --- → mozilla0.9.4
Seems silly to make HaveNewerMilestone() an NS_IMETHOD. Since it's an internal
function why not simplify life and make it return a boolean. It's not like you
really care about any errors returned -- go ahead and assume false unless
everything works out trying to compare the values.    if (HaveNewerMilestone())...

You get the pref service again inside HaveNewerMilestone, when the calling
function already has it. Sure, the service manager should be relatively quick
for a cached service, but it's a lot more overhead than simply passing in a pointer.

You could re-write the end of the routine to combine the two cases that write
the pref back out, would make the code a little more compact/readable 
Why not call it "GetHomePageOverride"?
Attached patch Patch revision 2Splinter Review
+PRBool nsBrowserContentHandler::GetHomepageOverride(nsIPref *aPrefSvc)

Shouldn't that be |const nsIPref* aPrefService|? (No need really to abbrv.,
don't you think?)

+  if (!aPrefSvc)
+    return PR_TRUE;

I don't think this is needed. Perhaps put an NS_ASSERTION in there, but since
the only user of this code already checks whether we have a prefs service, and
bails if we don't, I don't think that is even necessary.

+  nsCOMPtr<nsIHttpProtocolHandler> httpHandler(
+    do_GetService("@mozilla.org/network/protocol;1?name=http", &rv));

Indent this a little more, like:

+  nsCOMPtr<nsIHttpProtocolHandler> httpHandler(
+           do_GetService("@mozilla.org/network/protocol;1?name=http", &rv));

To make it look less like the ordinary code block indentation.

+  rv = aPrefSvc->GetCharPref(PREF_HOMEPAGE_OVERRIDE_MSTONE, 
+    getter_Copies(savedMilestone));

Per accepted style, arguments should line up, like:

+  rv = aPrefSvc->GetCharPref(PREF_HOMEPAGE_OVERRIDE_MSTONE, 
+                             getter_Copies(savedMilestone));


This:

+      !(*savedMilestone.get()) || 

is really an IsEmpty() check (you're testing if the first character is \0, in
other words an empty string). In this case though there is no need for that, the
.Equals will do the right thing.
Attached patch Patch revision 3Splinter Review
r=jag
r=dveditz in case a double review will speed up the super-review/approval
process any
Whiteboard: Have patch; need r, sr, a → Have patch, r; Need sr, a
Attachment #47701 - Flags: review+
sr=mscott
Whiteboard: Have patch, r; Need sr, a → Have patch, r, sr; Need a
Attachment #47701 - Flags: superreview+
a=blizzard on behalf of drivers for 0.9.4
Whiteboard: Have patch, r, sr; Need a → Have patch, r, sr, a
Attachment #47701 - Flags: approval+
Fix checked in 2001-08-31 afternoon.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
verified all platforms 200109050x
Status: RESOLVED → VERIFIED
Maybe it's just me, but this seems to be broken on Mac. It brings me to the
mozilla organisation page every time I start Mozilla. I haven't really looked
into it, but just making a note of it here.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.