Closed Bug 391380 Opened 16 years ago Closed 16 years ago

moz-icon: launches handler dialog


(Firefox :: File Handling, defect)

Not set



Firefox 3 alpha8


(Reporter: rflint, Assigned: sdwilsh)



(Keywords: regression)


(2 files)

Attached image Screenshot
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007080804 Minefield/3.0a8pre

The handler dialog now opens when viewing anything containing a moz-icon URI (download manager, main tab of preferences, file type manager, etc.).
This is quite ugly,  I'd almost call it a smoketest blocker, but it doesn't block any functionality, just tosses up that Launch Application dialog.

Seen while opening Downloads manager (or initiating a download) and opening the Preference window.
So, that makes sense because I'm pretty sure moz-icon isn't implemented on Linux.  Now, the question arises as to why you are getting that...
I'm seeing this as well... Having the SeaMonkey download manager popping up one such dialog per file listed in it makes an interesting dialog-click-away game :(
Does data:text/html,<img%20src="unknown:"> trigger the bug?
(In reply to comment #5)
> Does data:text/html,<img%20src="unknown:"> trigger the bug?

Doesn't seem to trigger it for me here...

Can someone help with a regression range? Must be very recently, in the last few days. And as this was reported in Firefox and I see it in SeaMonkey and can also trigger it in Thunderbird when viewing a message with an attachment, this must be a core regression.
nighly build 2007080704 works perfectly,
but build 2007080804 has the problem.

Hope that helped
Linus, that helps a lot, thanks. It narrows down the checkins to blame to something like
So what I think is going on here is that for some reason there's no moz-icon handler registered in the build (which is odd).

Then we go to load an <img src="moz-icon:....">.  And since we're in chrome, the patch for bug 388597 makes us skip content policy checks, which means we no longer hit nsNoDataProtocolContentPolicy::ShouldLoad (which used to block such loads to prevent popup dialogs).

What confuses me is that nsNoDataProtocolContentPolicy has only existed since 2006-06-26.  Did we use to pop up a dialog in these circumstances before that or something?
OK, the reason this isn't a problem with, say, 1.8 branch is that nsExternalProtocolHandler::NewChannel fails out because there's no handler for that type.

On trunk, on the other hand, nsExternalProtocolHandler::HaveExternalProtocolHandler calls nsExternalHelperAppService::ExternalProtocolHandlerExists.  This calls nsExternalHelperAppService::GetProtocolHandlerInfo, which calls nsOSHelperAppService::GetProtocolInfoFromOS.  This last always returns a handler info, which nsExternalHelperAppService::ExternalProtocolHandlerExists interprets to mean that there is a handler:

1141      nsCOMPtr<nsIHandlerInfo> handlerInfo;
1142      nsresult rv = GetProtocolHandlerInfo(
1143          nsDependentCString(aProtocolScheme), getter_AddRefs(handlerInfo));
1144      if (NS_SUCCEEDED(rv)) {
1145        *aHandlerExists = PR_TRUE;
1146        return NS_OK;
1147      }
I meant "that protocol", not "that type".
Attached patch v1.0Splinter Review
I still cannot reproduce this locally, but based on irc discussions, I think this should fix it.
Assignee: nobody → sdwilsh
Comment on attachment 275992 [details] [diff] [review]

sr=dmose with an added XXX comment to that we're gonna need to poke this again after the possibleHandlers patch lands.  Filing a spinoff bug on that would be good too.
Attachment #275992 - Flags: superreview+
Attachment #275992 - Flags: review?(robzare)
Attachment #275992 - Flags: review?(robzare) → review?(bzbarsky)
Comment on attachment 275992 [details] [diff] [review]

Looks good.

a=bzbarsky too.
Attachment #275992 - Flags: review?(bzbarsky)
Attachment #275992 - Flags: review+
Attachment #275992 - Flags: approval1.9+
Blocks: 389969
Flags: in-testsuite-
Flags: in-litmus?
Target Milestone: --- → Firefox 3 M8
Checking in uriloader/exthandler/nsExternalHelperAppService.cpp;
new revision: 1.333; previous revision: 1.332
Checking in modules/libpref/src/init/all.js;
new revision: 3.686; previous revision: 3.685
Closed: 16 years ago
Resolution: --- → FIXED
Litmus Triage Team: ctalbert, will you handle this test case?
You need to log in before you can comment on or make changes to this bug.