I modified nsWindowsShellService.cpp so it supports setting the app as the default and all registry values are properly set but the OS displays a dialog stating that the app is hung / must be closed. I suspect we will need to use a separate exe (NSIS?) as IE does to provide this.

Other reasons to use a separate exe to do this... when the profile manager is set to always display on startup the profile manager is displayed. when the app is upgraded but hasn't been started yet the extension compatibility wizard is displayed. with UAC turned on the process that is launched is given an elevated token. If the process is already running I believe this will fail. If by chance it doesn't fail the app is granted an elevated token which may be a security risk.

Taking... I have this partially completed.

Note: trying to set the app as the default for the OS is broken when using UAC and the app is already running presumably. I recall seeing an msdn article where I believe it stated that the exe's token would not be modified if the exe is already running. If I run across the article again I'll post a link in this bug unless of course someone else beats me to it.

s/presumably//

*** Bug 359643 has been marked as a duplicate of this bug. ***

Blocking for now, but incomplete Vista bugs might have to wait for 1.8.1.2

No longer blocking 1.8.1.1 as per recent discussion with drivers around Vista readiness requirements.

Doug, this patch only contains the OS Integration changes. I'm going to ask bsmedberg to review it if it passes the muster.

Comment on attachment 251157 [details] [diff] [review] patch (Installer) fyi: the string changes aren't part of the branch patch.

Comment on attachment 251156 [details] [diff] [review] patch -w (OS Integration) -w, i am sure that the spacing is better that it was and I am happy that more of this blames to you :-) what happened to the XPI key that was set in gSettings? in CheckArgShell(), the comment says that you need to use lowercase, but that isn't the case.' In nsAppRunner.cpp, i think you should move the two XP_WIN blocks into a single function. Also, i am somewhat nervous about the guesstimate test. Maybe this is something we can put in to a pref (not sure if all.js is available at this point or not). This would allow us to tweak this value in the field if need-be. I need to give CheckArgShell() a bit more of a look over tomorrow morning (or tonight). Why are you changing the mMutexName to include MOZ_MUTEX_NAMESPACE. Is this for vista or something? Just to be sure -- the argument passed to a xul app named "-requestPending" will only be used "internally" to restart the application. If this is the case, do you want to name it something more obscure?

(In reply to comment #12) > (From update of attachment 251156 [details] [diff] [review]) > ... > what happened to the XPI key that was set in gSettings? It wasn't used... it is my understanding it was something that was planned but never implemented. > in CheckArgShell(), the comment says that you need to use lowercase, but that > isn't the case.' will fix > In nsAppRunner.cpp, i think you should move the two XP_WIN blocks into a single > function. agreed > Also, i am somewhat nervous about the guesstimate test. Maybe this is > something we can put in to a pref (not sure if all.js is available at this > point or not). This would allow us to tweak this value in the field if > need-be. I was thinking of just bumping it up to around 100 instead of a pref. This code path will seldom be hit and the small amount of additional time should be fine when it is hit. > I need to give CheckArgShell() a bit more of a look over tomorrow morning (or > tonight). cool and thanks > Why are you changing the mMutexName to include MOZ_MUTEX_NAMESPACE. Is this > for vista or something? I almost left it out of the patch... it adds the namespace which will fallback to local when it isn't present... it is just good coding habit. > Just to be sure -- the argument passed to a xul app named "-requestPending" > will only be used "internally" to restart the application. If this is the > case, do you want to name it something more obscure? No, it is to tell the command line handler to not launch the url since a dde request will follow with the url.

(In reply to comment #12) ... > in CheckArgShell(), the comment says that you need to use lowercase, but that > isn't the case.' fyi: I just verified that it must be lowercase

addresses dougt's comments.

Comment on attachment 251297 [details] [diff] [review] v.2 patch -w (OS Integration) It would be really great to have a testsuite for this code... it's probably not especially susceptible to unit-testing, but an uber-complete Litmus testsuite.

bsmedberg: my plan is to have a Vista specific testsuite at the top level of Litmus, and this functionality will be included in my testing. (In reply to comment #16) > (From update of attachment 251297 [details] [diff] [review]) > It would be really great to have a testsuite for this code... it's probably not > especially susceptible to unit-testing, but an uber-complete Litmus testsuite. >

Comment on attachment 251297 [details] [diff] [review] v.2 patch -w (OS Integration) This is really good stuff. I mostly have nits, which I will not bother with. We need lots of edge case testing on the trunk. Rob, can you make sure that you give the community a heads-up. r=dougt

Comment on attachment 251157 [details] [diff] [review] patch (Installer) r=sspitzer we really need to get robert a pony for this one. rstrong sat down and explained this patch to me last week, and going over it with fresh eyes, I don't see any new issues. some issues he raised while explaining the patch to me: 1) should this code be disabled for thunderbird? + WriteRegStr SHCTX "$R5\shell\open\ddeexec" "" "$\"%1$\",,0,0,,,," + WriteRegStr SHCTX "$R5\shell\open\ddeexec" "NoActivateHandler" "" + WriteRegStr SHCTX "$R5\shell\open\ddeexec\Application" "" "${DDEApplication}" + WriteRegStr SHCTX "$R5\shell\open\ddeexec\Topic" "" "WWW_OpenURL" 2) do we have a spin off bug about the issue of "when firefox is launched from the installer (which has elevated privileges), does firefox have elevated privileges?" 3) Q: for the software update scenario, do we have a plan to update the registry to match what the installer does? (Yes, see bug #367165) 4) finally, there was one issue that robert raised about what happens with side-by-side installs and the registry on vista. I believe the decision was "the last install wins", but I forgot what this was in reference too. Robert, do you remember? (if so, can you log a spin off bug on that, even if we don't plan on addressing it?)

5) do we have a spin off bug about the issue of "when firefox is launched from updater.exe (which has elevated privileges), does firefox have elevated privileges?" as with the other spin off bug about elevated privs in comment #19, I believe the answer is yes, so i'll go look for those bugs.

bug 367540 for the installer launching the app with elevated privs bug 364483 for the updater launching the app with elevated privs

Seth, the combination patch you asked for

Checked in to trunk

i can still reproduce the problem i posted on https://bugzilla.mozilla.org/show_bug.cgi?id=359643 (which had been marked dup of this bug) Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a2pre) Gecko/20070130 Minefield/3.0a2pre ID:2007013001 [cairo]

This bug is to handle setting as the OS default (e.g. clicking a link in an email program, etc.) and I haven't looked at that bug. I'll take a look at bug 359643 as time permits and reopen that bug if I can reproduce, etc. It doesn't appear to be directly related to this though... also, some programs actually directly call IE on Windows which may be what you are seeing.

I can verify fixed- this with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a2pre) Gecko/2007013004 Minefield/3.0a2pre, its working fine with links as example from thunderbird and also with the Links inside the Avast! Program (Bug 359643 comment #2)

Please see http://wiki.mozilla.org/Firefox:1.5.0.10-2.0.0.2:Test_Plan:Vista_Focused_Testing for results of my Vista testing. I tested with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a2pre) Gecko/2007013004 Minefield/3.0a2pre. I had the most success in the scenario with QA machine 4, running with a fresh profile and a new install. Where things got mucky in my testing was the machines that had existing profiles/possible cruft lying around. Rob- is it expected that a user would have to restart the first time after setting the app as default?

(In reply to comment #27) > Please see > http://wiki.mozilla.org/Firefox:1.5.0.10-2.0.0.2:Test_Plan:Vista_Focused_Testing > for results of my Vista testing. I tested with Mozilla/5.0 (Windows; U; Windows > NT 6.0; en-US; rv:1.9a2pre) Gecko/2007013004 Minefield/3.0a2pre. I had the most > success in the scenario with QA machine 4, running with a fresh profile and a > new install. Where things got mucky in my testing was the machines that had > existing profiles/possible cruft lying around. There are no profile issues that I can think of but there are side by side issues and possibly upgrade issues... that is much more likely what is going on since you would have either had a side by side or an upgrade with an existing profile present. > Rob- is it expected that a user would have to restart the first time after > setting the app as default? How did you launch the app... from software update, from the installer, or from the shortcut?

App was launched from the installer, at which point I set it as the default browser. I did not test this scenario from software update or from the shortcut, but I can do that tomorrow when I am beside my Vista machines x4. (In reply to comment #28) > (In reply to comment #27) > > Please see > > http://wiki.mozilla.org/Firefox:1.5.0.10-2.0.0.2:Test_Plan:Vista_Focused_Testing > > for results of my Vista testing. I tested with Mozilla/5.0 (Windows; U; Windows > > NT 6.0; en-US; rv:1.9a2pre) Gecko/2007013004 Minefield/3.0a2pre. I had the most > > success in the scenario with QA machine 4, running with a fresh profile and a > > new install. Where things got mucky in my testing was the machines that had > > existing profiles/possible cruft lying around. > There are no profile issues that I can think of but there are side by side > issues and possibly upgrade issues... that is much more likely what is going on > since you would have either had a side by side or an upgrade with an existing > profile present. > > > Rob- is it expected that a user would have to restart the first time after > > setting the app as default? > How did you launch the app... from software update, from the installer, or from > the shortcut? >

(In reply to comment #29) > App was launched from the installer, at which point I set it as the default > browser. I did not test this scenario from software update or from the > shortcut, but I can do that tomorrow when I am beside my Vista machines x4. I believe that software update has the can't open link from other apps on launch after update bug that we saw this morning. Please first verify whether that is reproduceable and then check out if what you wrote in this bug is. Also try setting defaults on first run after software update and try clicking links from other apps on launch from the installer. I suspect both of these are due to being elevated on launch from both software update and the installer. Also, be sure to update from a build before the vista patches landed.

(In reply to comment #25) >some programs actually directly call IE on Windows which may be what you are >seeing. i tried testing it on windows xp. links on yahoo messenger launches the default browser, either ie or firefox (whichever the default browser i set). thus, yahoo messenger does not directly calling ie. same goes to free download manager (i haven't tried it yet with avats!)

Please take your comments over to bug 359643... as stated in comment #25 this is about OS Integration where you can click a http link in an email app, etc. and it opens the default browser. If you can do that which is inferred by your other comments and Yahoo IM is still not opening the Firefox when it is set as default then you are experiencing a different bug.

Since this check-in was made, clicking on a link from an external app with XP opens Minefield with my default homepage and an about:blank page. The desired link is not opened. If Minefield is already running, it will work properly however.

Mike: I don't see this using XP. I am using Thunderbird as the external app - with Minefield closed links open fine. Which external app(s) were you using? If it turns out others can reproduce this, will morph to a new bug so we don't clutter this one with a lot of noise. (In reply to comment #33) > Since this check-in was made, clicking on a link from an external app with XP > opens Minefield with my default homepage and an about:blank page. The desired > link is not opened. If Minefield is already running, it will work properly > however. >

Try verifying that the app is set as default in tools -> options -> main tab -> check now button

(In reply to comment #34) > Mike: I don't see this using XP. I am using Thunderbird as the external app - > with Minefield closed links open fine. also wfm with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a2pre) Gecko/20070118 Minefield/3.0a2pre ID:2007011804 [cairo] tried with the following apps: Skype MSN Messenger ICQ Thunderbird Avast

Confirming. In the instance I note in Comment 34, Minefield is indeed set as default when you investigate via "Check Now." (In reply to comment #35) > Try verifying that the app is set as default in tools -> options -> main tab -> > check now button >

(In reply to comment #34) > Mike: I don't see this using XP. I am using Thunderbird as the external app - > with Minefield closed links open fine. Which external app(s) were you using? If > it turns out others can reproduce this, will morph to a new bug so we don't > clutter this one with a lot of noise. It's every app that can launch a link (xchat, The Bat!, RSSBandit are just a few.) The has only happened since this patch landed, so I can't think of anything else that would cause this. What I have found since originally posting about this is, if I disable an extension - any extension - then restart Minefield, the first link launched will work. But, subsequently, everything else will just launch my homepage and about:blank. This may indeed be a different bug. I'm sorry if I'm causing undue clutter. Just fyi, this problem persists in: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070201 Minefield/3.0a2pre ID:2007020108 [cairo]

Comment on attachment 254128 [details] [diff] [review] Branch patch with everything plus the kitchen sink r=sspitzer

fix landed on branch.

this was missed on initial branch checkin and already has approvals

verified fixed for 1.8.1.2 using Build identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.2) Gecko/2007020823 Firefox/2.0.0.2 I`ve set 1.8.1.2 as default browser and links from other programs and start search and external apps are handled by firefox

note: I highly suspect that bug 380496 will have to be fixed on Oracle's end per bug 380496 comment #2

A few times when I closed firefox, I found it in the process explorer still running. An the command line was ""C:\Program Files\Mozilla Firefox\firefox.exe" -requestPending -osint -url "http://someforum/showthread.php?t=123&goto=newpost" This was an URL from thunderbird which I clicked. So it seems this starts a process which hangs then? I'm running windows 7.