Closed Bug 343602 Opened 15 years ago Closed 14 years ago

profile-startup-category notifications don't happen if profile is specified

Categories

(SeaMonkey :: Startup & Profiles, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ajschult784, Assigned: ajschult784)

Details

(Keywords: fixed-seamonkey1.1a)

Attachments

(2 files)

profile-startup-category notifications happen in nsProfile::LoadDefaultProfileDir, but that method is not called if the commandline included the profile (ProcessArgs would have called SetCurrentProfile).  It seems that the notifications should happen at the end of SetCurrentProfile.

LoadDefaultProfileDir also does pref conversion to UTF8, which seems like another thing that should live in SetCurrentProfile.
Attached patch patchSplinter Review
This just moves things from LoadDefaultProfileDir into SetCurrentProfile.
Assignee: nobody → ajschult
Status: NEW → ASSIGNED
Attachment #228494 - Flags: review?(benjamin)
Comment on attachment 228494 [details] [diff] [review]
patch

I'm sorry, I don't even kinda-sorta own this code and don't want to.
Attachment #228494 - Flags: review?(benjamin)
Attachment #228494 - Flags: review?(cbiesinger)
It seems this stuff is not used by toolkit apps
Component: Profile: BackEnd → Profile: Manager
Product: Core → Mozilla Application Suite
The only effect of this bug is that the update notification doesn't work.  But that seems like a serious enough problem to need this for 1.1
Flags: blocking-seamonkey1.1a?
Flags: blocking-seamonkey1.1a? → blocking-seamonkey1.1a+
Comment on attachment 228494 [details] [diff] [review]
patch

I don't know this code very well but it looks OK to me. Please don't add trailing whitespace on those new lines. (This does make the listener run on live profile switching, but that seems ok)
Attachment #228494 - Flags: review?(cbiesinger) → review+
Attachment #228494 - Flags: superreview?(neil)
(In reply to comment #5)
>This does make the listener run on live profile switching, but that seems ok)
No it doesn't, because the update notifier assumes it only starts once.
Hmm, can we get a fix into 1.8 branch within the next week, so we can get 1.1a out in time?
This makes the update notifier not notify twice after switching profiles.
Attachment #234338 - Flags: superreview?(neil)
Attachment #234338 - Flags: review?(neil)
Comment on attachment 234338 [details] [diff] [review]
fix update notifier

>-const kDebug               = false;
>+const kDebug               = true;
Did you really mean this? ;-)

>+    // Check if we've already been called.
>+    if (initialized)
>+      return;
>+    initialized = true;
You should use this.mInitialized etc, not a global. sr=me with this fixed.
Attachment #234338 - Flags: superreview?(neil)
Attachment #234338 - Flags: superreview+
Attachment #234338 - Flags: review?(neil)
Attachment #234338 - Flags: review+
Comment on attachment 228494 [details] [diff] [review]
patch

>+               nsCOMPtr <nsIProfileStartupListener> listener = do_CreateInstance(contractidString.get(), &rv);
Since we'll notify on a profile switch, we need do_GetService here.
Otherwise we'll create new update notifiers on each profile switch!
sr=me with this fixed.
Attachment #228494 - Flags: superreview?(neil) → superreview+
Comment on attachment 228494 [details] [diff] [review]
patch

a=me for 1.1 given it works on trunk.
Attachment #228494 - Flags: approval-seamonkey1.1a+
Comment on attachment 234338 [details] [diff] [review]
fix update notifier

a=me for 1.1 given it works on trunk.
Attachment #234338 - Flags: approval-seamonkey1.1a+
FIXED on trunk and branch
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
this exposed the fact that nsUpdateNotifier gets leaked (bug 349323)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
oops, meant to leave this fixed
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.