Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072722 Minefield/3.0a7pre ID:2007072722 repro: Open FF go to ttp://google.com (see Bug 389705) result: javascript Alertbox: Firefox doesn't know how to open this address, because the protocol isn't associated with any program. expected: the new "protocol handling dialog"

another case which seems to work for others download http://www.justin.tv/widgets/jtv_player.swf press open in the download manager result: see screenshot (Windows Dialog) expected: Firefox protocol handling dialog

I need to manually register a protocol (e.g. webcal) to get the protocol handling dialog

Does a nightly from before bug 385065 landed work the same, or is this a regression?

regressionrange: works in 20070726_1956_firefox-3.0a7pre.en-US.win32 fails in 20070727_1135_firefox-3.0a7pre.en-US.win32 http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=1185504960&maxdate=1185561299&cvsroot=%2Fcvsroot looks like the 2007-07-27 11:31pdt checkin broke this

OK, that is Bug 389632.

comment 1 mixes up protocol and content handling, I think. Open in the download manager opens the file, there's no protocol involved at that point, and we don't have web handlers for content types hooked up yet. comment 2 is a little confusing, since i don't remember how we'd handle a protocol that the OS didn't have a handler for, though I think it would be the same behaviour...

However, since protocol and mime type handling are implemented on the same object, it is possible that we broke something in both (comment 1). I'm not sure how the behavior in comment 2 is coming about...

When i submitted bug 389632, I had been looking at the code prior to Bug 385065 landing, and wasn't aware of those changes. Now nsExternalHelperAppService::ExternalProtocolHandlerExists maybe should always return true as the user will (always?) be asked for an application.

This hack at least indicates where code changes may need to be made. Thanks to Justin on bug 389766 for pointing out that nsMIMEInfoImpl::GetHasDefaultHandler is now what really says if there is a system specified handler. This provides a hack fix in GetHasDefaultHandler for that bug but Justin's suggestions in bug 389766 comment 7 are better. nsOSHelperAppService::GetProtocolInfoFromOS now should (usually) return non-null but, if the handler doesn't exist (or is disabled through gconf) it should not do anything that might make GetHasDefaultHandler set _retval to true. (Setting the description should be fine though if Justin's suggestions in bug 389766 are carried out.)

Some of the comments added in bug 389632 should be removed also, and we could probably remove the redundant check on HaveExternalProtocolHandler in nsExternalProtocolHandler::NewChannel.

Similar changes need to be made to other OS's too.

(In reply to comment #8) > Now nsExternalHelperAppService::ExternalProtocolHandlerExists maybe should > always return true as the user will (always?) be asked for an application. The user will usually be asked for an application, modulo the blacklist preferences, or so my understanding of the current UI design goes. In the short term, your suggestion might be the right one; I'm unsure. Over the longer term, my suspicion is that one of the paths towards making this code less fragile is likely to involve dis-entraining the front-end decisions from the backend ones. Exactly where to draw this line is likely to be the interesting question, but one could imagine the front-end making all relevant decisions about whether or not the user should be asked.

Comment on attachment 274450 [details] [diff] [review] working hack for unix >- (*aHandlerInfo)->SetPreferredAction(nsIHandlerInfo::useSystemDefault); >+ (*aHandlerInfo)->SetPreferredAction(nsIHandlerInfo::useSystemDefault); //XXXkt what if no system default? Check, and if we don't, set it to nsIHandlerInfo::alwaysAsk >- if (NS_FAILED(rv) || !exists) >+ if (NS_FAILED(rv) /* || !exists */) shouldn't we remove this bit instead of just commenting it out (or do some XXX comment stating why it is commented out Otherwise, I think this is right for M7

Not so hackish way - does what dmose and I just talked about.

addresses comments

Comment on attachment 274484 [details] [diff] [review] v1.1 sr=dmose

Comment on attachment 274484 [details] [diff] [review] v1.1 please add a comment to the IDL saying that alwaysAsk basically means not initialized uriloader/exthandler/nsMIMEInfoImpl.cpp + *_retval = !mDefaultAppDescription.IsEmpty();; remove one of the semicolons uriloader/exthandler/beos/nsOSHelperAppService.h wrong indentation here

for checkin

typo free for checkin

Checking in uriloader/exthandler/nsExternalHelperAppService.cpp; new revision: 1.332; previous revision: 1.331 Checking in uriloader/exthandler/nsExternalHelperAppService.h; new revision: 1.85; previous revision: 1.84 Checking in uriloader/exthandler/nsMIMEInfoImpl.cpp; new revision: 1.64; previous revision: 1.63 Checking in uriloader/exthandler/beos/nsOSHelperAppService.cpp; new revision: 1.24; previous revision: 1.23 Checking in uriloader/exthandler/beos/nsOSHelperAppService.h; new revision: 1.14; previous revision: 1.13 Checking in uriloader/exthandler/mac/nsOSHelperAppService.cpp; new revision: 1.54; previous revision: 1.53 Checking in uriloader/exthandler/mac/nsOSHelperAppService.h; new revision: 1.19; previous revision: 1.18 Checking in uriloader/exthandler/unix/nsOSHelperAppService.cpp; new revision: 1.71; previous revision: 1.70 Checking in uriloader/exthandler/unix/nsOSHelperAppService.h; new revision: 1.24; previous revision: 1.23 Checking in uriloader/exthandler/win/nsOSHelperAppService.cpp; new revision: 1.75; previous revision: 1.74 Checking in uriloader/exthandler/win/nsOSHelperAppService.h; revision: 1.29; previous revision: 1.28 Checking in netwerk/mime/public/nsIMIMEInfo.idl; new revision: 1.32; previous revision: 1.31

Tested Fix on Linux (Ubuntu 7.04) and Windows XP. On both those operating systems, the fix looks good. Content handling falls through to the OS, as expected, and for unknown protocol handlers the protocol selection dialog is shown. But, on Mac OS X, the protocol dialog is not shown, and the steps from Comment 0 still reproduce the bug. Tested with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/2007073016 Minefield/3.0a7pre (First hourly build to contain the fix). Also, get this error message in the console window: Error: Warning: unrecognized command line flag -foreground Source File: file:///Volumes/Minefield/Minefield.app/Contents/MacOS/components/nsBrowserContentHandler.js Line: 647 Sdwilsh believes the problem to be here: http://mxr.mozilla.org/seamonkey/source/uriloader/exthandler/mac/nsOSHelperAppService.cpp#99 --> REOPENING bug

Looks like the problem is really here: http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/mac/nsInternetConfigService.cpp#128 The HasProtocolHandler method simply returns NS_ERROR_FAILURE when it really should be returning NS_OK. This ends up getting propagated a couple of frames up the stack and triggering the "return nsnull" here: http://mxr.mozilla.org/seamonkey/source/uriloader/exthandler/mac/nsOSHelperAppService.cpp#326

Here's a semi-minimal fix for the Mac problem. It makes nsInternetConfigService::HasProtocolHandler tell the truth about whether or not there's a handler for the given protocol, although it continues to notify its caller if the handler happens to be the current app. Then, in nsOSHelperAppService::OSProtocolHandlerExists, instead of displaying an alert if the handler happens to be the current app, we pretend there's no handler for the given protocol. A more robust fix would pass word of the handler loop back to the caller of OSProtocolHandlerExists, which would pass it back to its caller, and all the way up the chain until we get to the front-end code, which would either automatically resolve the problem by breaking the loop somehow or notify the user about the problem and provide the user with options for breaking the loop. But that robust fix seems out of scope for M7 and may well be out of scope for Fx3 overall. In any case, this semi-minimal fix resolves the current issue and makes the code a bit more transparent about what it's doing, so if we decide to implement the more robust fix later on (or just forget about it and someone excavates this code later) it'll be clearer what's going on and what needs to be done.

biesi and I discussed on IRC that it would make more sense for HasProtocolHandler to leave its out param false when the handler is the current app, since it's "unusual to have usable out params in a failure return", and since we ignore the out param anyway when we see the failure at the moment. Here's a version that does it like that. If nsInternetConfigService::HasProtocolHandler returns NS_ERROR_NOT_AVAILABLE, then it doesn't try to set the out param to specify that there's a handler, and nsOSHelperAppService::OSProtocolHandlerExists doesn't assume anything about the value of the out param.

Comment on attachment 274570 [details] [diff] [review] patch v2: minimal fix that doesn't assume usable out param on failure return sr=dmose. very nice; excising code is great!

a=shrep for followup patch per blanket approval for bugs on wiki page and bug list of M7 blockers Checked in the followup patch: Checking in uriloader/exthandler/mac/nsInternetConfigService.cpp; /cvsroot/mozilla/uriloader/exthandler/mac/nsInternetConfigService.cpp,v <-- nsInternetConfigService.cpp new revision: 1.42; previous revision: 1.41 done Checking in uriloader/exthandler/mac/nsOSHelperAppService.cpp; /cvsroot/mozilla/uriloader/exthandler/mac/nsOSHelperAppService.cpp,v <-- nsOSHelperAppService.cpp new revision: 1.55; previous revision: 1.54 done

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007073022 Minefield/3.0a7pre ID:2007073022 Error: (void 0) has no properties Source file: chrome://global/content/bindings/dialog.xml Line: 81 if the "Launch Application Dialog" is openend (gotta dash to work so can't file a new bug)

(In reply to comment #27) bah! I hit that at one point when working on this, then it went away and I couldn't reproduce it... I'm not 100% sure what's up with it.

Verified fixed on the Mac with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/200707310404 Minefield/3.0a7pre. With the steps in Comment #0, I get asked on the Mac to pick an app to handle the protocol.

related: "launch application dialog loses resizing state when checkbox is clicked" https://bugzilla.mozilla.org/show_bug.cgi?id=390555

This is regressed again: works in 20070809_1507_firefox-3.0a8pre.en-US.win32.zip fails in 20070809_1524_firefox-3.0a8pre.en-US.win32.zip http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1186697220&maxdate=1186698239 bug 391380?

> This is regressed again: For what protocol?

There is no default webcal handler on WinXP, so Firefox displays an alert "Firefox doesn't know how to open this address, because the protocol (webcal) isn't associated with any program". Tested on http://people.mozilla.org/~ctalbert/test-webcal-protocol.html

this is definitly broken. Do we need a new bug ?

(In reply to comment #34) > this is definitly broken. > Do we need a new bug ? > No, This is the same issue as when it was first reported. In this case we should reopen this bug. --> REOPENED

(In reply to comment #35) > (In reply to comment #34) > > this is definitly broken. > > Do we need a new bug ? > No, This is the same issue as when it was first reported. In this case we > should reopen this bug. Except that something else caused this to happen again, so I think we should file a new bug.

Ok, per comments here and on IRC, we have decided it would be better to open a new bug for this regression since this one is already pretty unwieldy. The new bug is bug 392964. I have added relevant parts of this bug and the regression information there, please add follow on comments and patches to that bug. Resetting this bug back to RESOLVED FIXED.

Litmus Triage Team: ctalbert, will you handle this Litmus case?