Closed Bug 476988 Opened 11 years ago Closed 11 years ago

Seamonkey Trunk crashes just after profile selection panel display, when Sm is invoked via internet shortcut

Categories

(SeaMonkey :: General, defect)

x86
Windows XP
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
seamonkey2.0b1

People

(Reporter: World, Assigned: mcsmurf)

Details

(Keywords: crash)

Attachments

(1 file, 3 obsolete files)

> Semonkey trunk 2009/2/03 build on MS Win-XP SP3
> [App] Vendor=Mozilla, Name=SeaMonkey, Version=2.0a3pre, BuildID=20090203001332
> [Gecko] MinVersion=1.9.1b3pre, MaxVersion=1.9.1b3pre

Seamonkey Trunk crashes just after profile selection panel display, if (a) Sm is set as default browser, (b) Sm is not active, (c) Sm is invoked via internet shortcut, (d) multiple profiles exist.

Crash ID: bp-157c446e-beb8-4480-892b-9a4542090204

Note: This is multiple profile version of Bug 461501(same scenario. defference is "multi-prof" only).
Bug 461501 has been fixed, and bug Bug 461501 doesn't occur with newest build. But crash of this bug still occurred.
  Crash ID: bp-b85a6ce1-3ee3-428c-9b49-430a42090208
In addition to above (a) to (d), (e) Uncheck of "Don't ask at startup" was required to produce crash.

(tested build)
> Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.9.1b3pre) Gecko/20090208 SeaMonkey/2.0a3pre
> about:buildconfig
> Source
> Built from http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c8099a4c91c3
> Build platform
> target
> i686-pc-mingw32
Keywords: crash
We may need to copy the fix from Bug 370123.
Attached patch Patch (obsolete) — Splinter Review
Ok, this is just the the patch from Firefox ported to SeaMonkey code. I don't think we actually need the #ifdef, though it currently looks like only Windows is affected, but the missing #ifdef should not do any damage on other platforms.
Frank, did you forget to set the review flags?
Attached patch Better patch (obsolete) — Splinter Review
We do not need the while loop as the normal -url http://www.foo.com arguments are no problem, the DDE calls via the commandline handler are. And those can only pass one url at a time.
Assignee: nobody → bugzilla
Attachment #362336 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #368817 - Flags: review?(neil)
Comment on attachment 368817 [details] [diff] [review]
Better patch

I guess this doesn't normally bite because if the profile manager is showing you would get a "FooApp is running, but not responding" prompt?

>+    if (!this._haveProfile) {
>+      try {
>+        // This will throw when a profile has not been selected.
>+        var fl = Components.classes["@mozilla.org/file/directory_service;1"]
>+                           .getService(Components.interfaces.nsIProperties);
>+        var dir = fl.get("ProfD", Components.interfaces.nsILocalFile);
>+        this._haveProfile = true;
>+      } catch (e) {
>+        cmdLine.handleFlagWithParam("url", false);
>+        cmdLine.preventDefault = true;
>+      }
>+    }
So you're saying that we can only enter this codepath without having a profile in the case of DDE when the profile manager is active? In which case, how about:
} catch (e) {
  cmdLine.preventDefault = true;
  return;
}
Attached patch Patch (obsolete) — Splinter Review
Ok, your suggestion works.
When the profile manager is already running and you click on an Internet Shortcut, you get "http://www.example.com could not be found ..." (error message from Windows).
Attachment #368817 - Attachment is obsolete: true
Attachment #369140 - Flags: review?(neil)
Attachment #368817 - Flags: review?(neil)
Comment on attachment 369140 [details] [diff] [review]
Patch

>+  _haveProfile: false,
Might it not be worthwhile caching this?

>   /* nsICommandLineHandler */
>   handle: function handle(cmdLine) {
>@@ -345,6 +346,25 @@
>       }
>     }
> 
>+    // If we don't have a profile selected yet (e.g. the Profile Manager is
>+    // displayed) we will crash if we open an url and then select a profile. To
>+    // prevent this handle all url command line flag and set the command line's
>+    // preventDefault to true to prevent the display of the ui. The initial
>+    // command line will be retained when nsAppRunner calls LaunchChild though
>+    // urls launched after the initial launch will be lost.
Could do this check first, no?

>+      } catch (e) {
>+        cmdLine.preventDefault = true;
>+        return;
>+      }
Hmm, I wonder whether it would be better to throw NS_ERROR_ABORT instead.
Attachment #369140 - Flags: review?(neil) → review+
This patch was checked in. Changed the patch to not cache the state of the profile manager and to throw instead of return.
Attachment #369140 - Attachment is obsolete: true
Pushed to comm-central, changeset 0ee1c44572f5.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
Comment on attachment 375423 [details] [diff] [review]
Patch (checked in)

>+      throw NS_ERROR_ABORT;

I fixed this mistake with changeset e3728d17c619 by changing this to "throw Components.results.NS_ERROR_ABORT".
Checked with next build. Problem was no reproduced. => VERIFIED.
> Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.9.1b5pre) Gecko/20090502 SeaMonkey/2.0b1pre
> about:buildconfig
> Built from http://hg.mozilla.org/releases/mozilla-1.9.1/rev/55fddd2e1b17
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.