382762 - Crash reporter client app should be named other than "crashreporter"

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Along the lines of bug 382665, the crash reporter client app should have a name other than "crashreporter"; it looks very unprofessional and un-Mac-like in the Dock.  (And, since this app has interactive UI, people are going to be seeing it in their Docks much more than updater, which runs only very briefly and non-interactively.)

I'd suggest simply "Crash Reporter", but "Mozilla Crash Reporter" or "$PRODUCTNAME Crash Reporter" would work, too.

I note that this app has a serious identity crisis; its appmenu is "Minefield Crash Reporter" (as is the report-sending window title), while the Hide and Quit items are Hide/Quit "Crash Reporter", and the app/bundle name and name displayed by the Dock are "crashreporter".

Comment 1

[Jesse Ruderman]

It should not be called "Crash Reporter", because that's what the thing built into OS X is called.  "Mozilla Crash Reporter" or "$PRODUCTNAME Crash Reporter" would be fine.

Comment 2

[Benjamin Smedberg]

Can we programmatically control what's displayed in the dock? Because this ships as part of the toolkit, we can't customize the bundle for each application.

Comment 3

[Colin Barrett [:cbarrett]]

There is an API to set your dock icon programmatically, yes. We can't set what name is displayed in the dock though, since it just takes the name of the application.

Why would you need to customize the bundle for each application, though? Wouldn't it just be called "Mozilla Crash Reporter" or what not and have an icon that represents it?

Furthermore, I disagree with Jesse that we shouldn't call it Crash Reporter because that's what's built in to OS X. The Apple Crash Reporter does not 1) show a dock icon and 2) does not have the same UI as our crash reporter. I don't see how anyone would confuse the two.

Comment 4

[(not currently active) Ted Mielczarek]

We shouldn't call it "Mozilla Crash Reporter" because we're going to ship this code with XULRunner, and it's going to confuse people if Joost crashes and pops up "Mozilla Crash Reporter".

Comment 5

[Colin Barrett [:cbarrett]]

(In reply to comment #4)
> We shouldn't call it "Mozilla Crash Reporter" because we're going to ship this
> code with XULRunner, and it's going to confuse people if Joost crashes and pops
> up "Mozilla Crash Reporter".

And there's really no PRODUCT_NAME variable at build time in toolkit? "Firefox Crash Repoter" and "Joost Crash Reporter" seem like pretty good names to me.

Comment 6

[Benjamin Smedberg]

There is no "build time"... the whole premise is that you're using the same binary across multiple applications.

Comment 7

[(not currently active) Ted Mielczarek]

So, all arguments about naming aside, if we at least wanted to make this say "Crash Reporter", where the heck would we do that?  Someone more apple-knowledgeable than me care to provide a hint (or a patch)?

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: patch

Since I can't build the breakpad stuff, this officially "untested", but this patch should do it, based on what I'm read from the dev docs and some local hacking of an existing crashreporter bundle for testing.   

The first two keys need to match the "physical" name of the bundle, and then LaunchServices will look up the key in the InfoPlist.strings and use that value for the Dock name and appname-menu name.  The LSHasLocalizedDisplayName key is supposed to speed things up, so that LaunchServices knows right away to go looking for keys.

This will also allow, say, the French versions to call the app "Journaliste d'accident" or whatever, if desired.

(If we want to fix the updater app, these same things should be done.)

Dunno who's the best reviewer here.  If you want to test this locally, you might have to make LaunchServices update its database for the crashreporter app (though re-making the app should cause the LS update).

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 273868 [details] [diff] [review]
patch

mento, luser said to ask you for review here :)

Comment 10

[Mark Mentovai]

Comment on attachment 273868 [details] [diff] [review]
patch

Why are the names in Info.plist and InfoPlist.strings out of sync?

Do we want to get rid of %APP_NAME% in light of the XULRunner thing mentioned above?

Comment 11

[Mark Mentovai]

Comment on attachment 273868 [details] [diff] [review]
patch

Smokey and I talked about some stuff here.  I still think we should drop %APP_NAME% in light of comments 4-6.

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: patch without %APP_NAME%

This removes %APP_NAME% and per the discussion with mento on irc, switches LSHasLocalizedDisplayName to a bool (Apple's docs are ambiguous; both string/1 and bool/true work, but bool shows up in more of the docs and is what Apple's apps use, so...).

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

According to the clear-as-mud checkin rules, this should be OK to land.

Comment 14

[Reed Loden [:reed]]

Checking in Info.plist;
/cvsroot/mozilla/toolkit/crashreporter/client/macbuild/Contents/Info.plist,v  <--  Info.plist
new revision: 1.3; previous revision: 1.2
done
Checking in Resources/English.lproj/InfoPlist.strings.in;
/cvsroot/mozilla/toolkit/crashreporter/client/macbuild/Contents/Resources/English.lproj/InfoPlist.strings.in,v  <--  InfoPlist.strings.in
new revision: 1.3; previous revision: 1.2
done

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 278114 [details] [diff] [review]
patch without %APP_NAME%

Just in case it's not clear from the previous comment, this patch has changes mento wanted to the previous patch (which we discussed on irc when he r+ the first patch); they had r=mento on irc that day.  Carrying forward the r+ to (hopefully) end the confusion.