Last Comment Bug 606998 - "Helper Applications" panel in preferences window unusable on a "non Gnome" system
: "Helper Applications" panel in preferences window unusable on a "non Gnome" s...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: seamonkey2.1b2
Assigned To: Manuel Reimer
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-25 10:16 PDT by Manuel Reimer
Modified: 2011-01-11 09:58 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix (1.62 KB, patch)
2010-10-25 10:17 PDT, Manuel Reimer
no flags Details | Diff | Splinter Review
Second patch (1.64 KB, patch)
2010-10-26 02:12 PDT, Manuel Reimer
no flags Details | Diff | Splinter Review
Patch, fixing the check in nsGNOMEShellService::Init (1.02 KB, patch)
2010-11-03 09:31 PDT, Manuel Reimer
neil: review+
neil: superreview+
Details | Diff | Splinter Review

Description Manuel Reimer 2010-10-25 10:16:41 PDT
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 2010-10-25 10:17:17 PDT
Created attachment 485765 [details] [diff] [review]
Fix
Comment 2 neil@parkwaycc.co.uk 2010-10-25 14:09:46 PDT
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 2010-10-26 02:12:14 PDT
Created attachment 486021 [details] [diff] [review]
Second patch
Comment 4 Manuel Reimer 2010-10-26 02:29:59 PDT
> 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 neil@parkwaycc.co.uk 2010-10-26 03:42:36 PDT
(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 2010-10-26 12:21:28 PDT
(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 2010-11-03 09:31:02 PDT
Created attachment 487928 [details] [diff] [review]
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 2010-11-05 11:37:34 PDT
Asking for "blocking SM 2.1" as I think we should ship SeaMonkey with a working "Helper Applications" pane.
Comment 9 Philip Chee 2010-11-10 09:55:11 PST
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/57f863e0caad
Comment 10 Jens Hatlak (:InvisibleSmiley) 2011-01-11 09:58:02 PST
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.

Note You need to log in before you can comment on or make changes to this bug.