606998 - "Helper Applications" panel in preferences window unusable on a "non Gnome" system

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Manuel Reimer]

I've set up a plain KDE system and getting following errors in error console:

Error: prefpane.paneload: loadSubScript("chrome://communicator/content/pref/pref-applications.js") failed:
[Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: chrome://communicator/content/pref/pref-applications.js :: <TOP_LEVEL> :: line 80"  data: no]
Source File: chrome://communicator/content/bindings/prefwindow.xml
Line: 563

I tracked this down to the following line of code:
http://mxr.mozilla.org/comm-central/source/suite/shell/src/nsGNOMEShellService.cpp#64

In my case *both* (gconf *and* vfs) are unavailable.

I've attached a patch against pref-applications.js to wrap the service generation into a "try/catch".

Comment 1

[Manuel Reimer]

Attachment: Fix

Comment 2

[[e-mail]]

Comment on attachment 485765 [details] [diff] [review]
Fix

Would you mind putting the try/catch inside the if, rather than outside?

I wonder whether it's better to remove the test for gconf/vfs in the shell service init routine, and add it in those methods that need it as and when.

Comment 3

[Manuel Reimer]

Attachment: Second patch

Comment 4

[Manuel Reimer]

> I wonder whether it's better to remove the test for gconf/vfs in the shell
> service init routine, and add it in those methods that need it as and when.

At least nsGNOMEShellService.cpp just checks for them, but never seems to actually use those variables...

I commented the whole block (check for gconf and vfs) and SeaMonkey compiled without errors.

Then I had a look at the nsGNOMEShellService.cpp used by firefox and theirs is much longer. Maybe we should copy this over and make our code work with this file?
http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/shell/src/nsGNOMEShellService.cpp

Comment 5

[[e-mail]]

(In reply to comment #4)
> Then I had a look at the nsGNOMEShellService.cpp used by firefox and theirs is
> much longer. Maybe we should copy this over and make our code work with this
> file?
> http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/shell/src/nsGNOMEShellService.cpp

There is already a bug for that... I'd started work on it, but the trouble was that the code that uses the shell service is all-or-nothing, so you can't easily implement (say) set as background first, and then something else later.

Comment 6

[Robert Kaiser]

(In reply to comment #2)
> I wonder whether it's better to remove the test for gconf/vfs in the shell
> service init routine, and add it in those methods that need it as and when.

That actually sounds like a good idea to me.

Comment 7

[Manuel Reimer]

Attachment: Patch, fixing the check in nsGNOMEShellService::Init

For our "limited" nsGNOMEShellService.cpp, the fix is as simple as the one, I've attached. I dropped the unused gconf and vfs variables and the check itself. Compiles well and makes the helper application prefpane work on my KDE-only system.

Comment 8

[Manuel Reimer]

Asking for "blocking SM 2.1" as I think we should ship SeaMonkey with a working "Helper Applications" pane.

Comment 9

[Philip Chee]

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/57f863e0caad

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

Wouldn't it make sense to also remove the header includes? They seem to be unused now:
#include "nsIGConfService.h"
and
#include "nsIGnomeVFSService.h"

FTR: FF did their fix for a similar problem in Bug 624267, differently.