395961 - Shouldn't show full path to handler executable in Launch Application dialog (show 'pretty name' instead)

Status: VERIFIED FIXED

(Firefox :: File Handling, defect, P2)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007091204 Minefield/3.0a8pre

Summary: Shouldn't show full path to handler executable in Launch Application dialog (show 'pretty name' instead)

Steps to Reproduce:

1. Click on http://phobos.apple.com/WebObjects/MZStore.woa/wa/storeFront
2. Note the resulting dialog that comes up (attached)

Expected Results:

I think we should show the pretty name, instead ("iTunes")

Actual Results:

See screenshot, which shows that we display the full path name to the application's executable, even with its arguments

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot

Comment 2

[Myk Melez [:myk] [@mykmelez]]

I wonder if we're getting that ugly name from the OS.

Comment 3

[Mike Connor [:mconnor]]

Isn't this a dupe?  I could swear I approved a patch for something really similar in the last week or so.

Comment 4

[Jim Mathies [:jimm]]

If it's the patch I think it is, that was in application prefs for helper apps. This looks like a path cleanup problem someplace in the helper app / mime info stuff.

Comment 5

[Jim Mathies [:jimm]]

I've been doing some work in nsOSHelperAppService, and noticed the implementation of GetApplicationDescription (overidden in /win) falls back on the full path in cases where GetDefaultAppInfo failes to find the target file. This must be the cause of what's happening here.

Comment 6

[Jim Mathies [:jimm]]

Hey Stephen, is this a reproducible bug that occurs on a common file type on all systems, or was this a onetime thing on a single machine?

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #6)
> Hey Stephen, is this a reproducible bug that occurs on a common file type on
> all systems, or was this a onetime thing on a single machine?

This occurs on both Windows XP and Vista, in fresh Virtual Machines.

Comment 8

[Jim Mathies [:jimm]]

The patch in 397678 should address this.

Comment 9

[Jim Mathies [:jimm]]

Removing dependency on bug 397678. See comments there for details - this is more critical than the Vista stuff.

Comment 10

[Jim Mathies [:jimm]]

Attachment: cleanup cmd handler path patch v.1

This should work much better than the previous routine and also sets things up for bug 397678.

I haven't come across the use of anything besides a dll or exe for a handler, but if we did find that it would be easy to add additional cases.

Comment 11

[Jim Mathies [:jimm]]

(Note, GetDefaultAppInfo didn't change I just moved it.)

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 300425 [details] [diff] [review]
cleanup cmd handler path patch v.1

Looks good! I'd appreciate additional comments regarding why we have to do it this way so it is obvious that this handles the case where the registry entry is not per the msdn spec.

Comment 13

[Jim Mathies [:jimm]]

Attachment: cleanup cmd handler path patch v.2

added commenting, and added support for .cpl which is used for windows card spaces. (downloaded .cpd files).

I ran a test app with this parsing functionality across every command handler in my laptop reg - it really did a nice job of parsing these things up. The older code didn't do as well.

Comment 14

[Jim Mathies [:jimm]]

Attachment: cleanup cmd handler path patch v.3

- addressed rare 'rundll32' entries on 2K
- split out functionality in CleanupCmdHandlerPath into two functions with better commenting
- addressed rare occurance of quotes around rundll handler: ["rundll32.exe" -foo]
- tested on Vista/XP/2K

Comment 15

[Jim Mathies [:jimm]]

This should land for Fx3. It's a major UE bug. If you look at the screen shot of iTunes I think you'll see how important it is.

Comment 16

[Jim Mathies [:jimm]]

Thanks biesi!

Comment 17

[Jim Mathies [:jimm]]

ok, already has blocking-firefox3 so I guess were ok to go.

Comment 18

[Reed Loden [:reed]]

Checking in uriloader/exthandler/win/nsOSHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/win/nsOSHelperAppService.cpp,v  <--  nsOSHelperAppService.cpp
new revision: 1.83; previous revision: 1.82
done

Comment 19

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041907 Minefield/3.0pre