824272 - Get the executable path directly from the directory service instead of guesswork.

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Philip Chee]

Firefox References:
Bug 791694 - Use XRE_EXECUTABLE_FILE in browser shell instead of guesswork from NS_XPCOM_CURRENT_PROCESS_DIR and MOZ_APP_NAME.
Bug 789469 - Get the executable path directly from the directory service instead of guesswork in WindowsJumpLists.jsm.

Comment 1

[Philip Chee]

Attachment: Patch 1.0 WIP

This patch fixes:
1. Shell Service (Windows and Unix)
2. FeedWriter.js
2b. Stop pre-processing FeedWriter.js
3. pref-applications.js
4. Windows Jumplists

feedback requests:
IanN for pref-applications.js
Neil for Shell Service and FeedWriter.js
mcsmurf for Windows Jumplists.

Comment 2

[Philip Chee]

Comment on attachment 695231 [details] [diff] [review]
Patch 1.0 WIP

> This patch fixes:
> 1. Shell Service (Windows and Unix)
> 2. FeedWriter.js
> 2b. Stop pre-processing FeedWriter.js
> 3. pref-applications.js
> 4. Windows Jumplists
> 
> feedback requests:
> IanN for pref-applications.js
> Neil for Shell Service and FeedWriter.js
> mcsmurf for Windows Jumplists.

Oh Stefan, can you check that the Feedwriter and applications prefpane changes are working correctly? Thanks.

Comment 3

[Stefan [:stefanh]]

Hmm, it looks like feed preview and the feed stuff in the helper apps pane is quite broken on mac... this patch doesn't seem to change anything, so I guess it's OK.

For the record (I have SeaMonkeyDebug set as the default app for feeds, tested with fresh profiles):

Helper apps pane
----------------
1) When I open the helper app prefpane I have "Preview in SeaMonkey" set for Podcast, Video Podcast and Web Feed. Opening the list, I can choose "Subscribe in SeaMonkey", "Use SeaMonkeyDebug (Default)" and "Use other..." for all 3 of the "feeds".

2) For fun, I added Safari to Podcasts - I then got it in the menulist, but I can't remove it by selecting "Application details". I can remove it from the list in the dialog, but nothing happens when I hit the OK button so the only option is to hit Cancel...

Feed preview (http://planet.mozilla.org/atom.xml)
------------
3) I have the following choices in the "Subscribe to this feed using" list:
  - "News & Blogs", "Live Bookmarks", "SeaMonkeyDebug", "Choose application...",
    "Google" and "My Yahoo!"
4) It's possible to choose "SeaMonkeyDebug", hitting "Subscribe Now" will then launch a new instance of SeaMonkeyDebug.

I should file a bug - possible multiple bugs, but before I do that I'd appreciate some advice on how to sort this out.

Comment 4

[Stefan [:stefanh]]

Comment on attachment 695231 [details] [diff] [review]
Patch 1.0 WIP

As mentioned previously, the patch doesn't change any behaviour on Mac. It's obviously beyond the scope of this bug to sort out the Mac issues.

Comment 5

[[e-mail]]

Comment on attachment 695231 [details] [diff] [review]
Patch 1.0 WIP

>@@ -300,22 +301,22 @@ nsresult
> GetHelperPath(nsString& aPath)
> {
>   nsresult rv;
>   nsCOMPtr<nsIProperties> directoryService = 
>     do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   nsCOMPtr<nsIFile> appHelper;
>-  rv = directoryService->Get(NS_XPCOM_CURRENT_PROCESS_DIR,
>+  rv = directoryService->Get(XRE_EXECUTABLE_FILE,
>                              NS_GET_IID(nsIFile),
>                              getter_AddRefs(appHelper));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  rv = appHelper->AppendNative(NS_LITERAL_CSTRING("uninstall"));
>+  rv = appHelper->SetNativeLeafName(NS_LITERAL_CSTRING("uninstall"));
This bit makes no sense to me because you don't care what the application leaf name is so you might as well stick with the application directory in the first place. (I meant to find someone on IRC to ask about that but it slipped my mind. Sorry about that.) f=me on the rest.

Comment 6

[Frank Wein [:mcsmurf]]

Comment on attachment 695231 [details] [diff] [review]
Patch 1.0 WIP

WindowsJumpLists change looks fine.

Comment 7

[Philip Chee]

Attachment: Patch v1.1 fix nits.

Changes since the last patch.

1. Remove unnecessary defines from Makefiles.

> This bit makes no sense to me because you don't care what the application
> leaf  name is so you might as well stick with the application directory in
> the first  place. (I meant to find someone on IRC to ask about that but it
> slipped my mind. Sorry about that.) f=me on the rest.

2. Removed changes in nsWindowsShellService.cpp.

Comment 8

[[e-mail]]

Comment on attachment 702248 [details] [diff] [review]
Patch v1.1 fix nits.

>-#expand    aExecutable.leafName != "__MOZ_APP_NAME__-bin";
Eek! This has been broken since back when we switched away from using -bin in 2.10 (?) - how far back can we port this patch?

Comment 9

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/1f96372e8f46