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

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[WADA:World Anti-bad-Duping Agency]

> 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).

Comment 1

[WADA:World Anti-bad-Duping Agency]

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

Comment 2

[Frank Wein [:mcsmurf]]

We may need to copy the fix from Bug 370123.

Comment 3

[Frank Wein [:mcsmurf]]

Attachment: Patch

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.

Comment 4

[Philip Chee]

Frank, did you forget to set the review flags?

Comment 5

[Frank Wein [:mcsmurf]]

Attachment: Better patch

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.

Comment 6

[[e-mail]]

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;
}

Comment 7

[Frank Wein [:mcsmurf]]

Attachment: Patch

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).

Comment 8

[[e-mail]]

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.

Comment 9

[Frank Wein [:mcsmurf]]

Attachment: Patch (checked in)

This patch was checked in. Changed the patch to not cache the state of the profile manager and to throw instead of return.

Comment 10

[Frank Wein [:mcsmurf]]

Pushed to comm-central, changeset 0ee1c44572f5.

Comment 11

[Frank Wein [:mcsmurf]]

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".

Comment 12

[WADA:World Anti-bad-Duping Agency]

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