Closed Bug 411490 Opened 13 years ago Closed 13 years ago
shaver suggested that we have an "about:crashes" page as an easy (but unintrusive) way to find recently submitted crash reports. Mossop already implemented the guts of this in Nightly Tester Tools, it might be nice to ship something built into firefox to make it easy for bug reporters to find their crashes.
I have this basically done
Status: NEW → ASSIGNED
Be nice to have in Thunderbird, too, except that it would need UI to open it, and somehow "View - Crashes" doesn't seem like a good menuitem.
This would be an invaluable help to testers and people who want to help with the debugging and problem solving process.
Here is the implementation for about:crashes. This is just implemented as a straight webpage with chrome privs. Uses the styling from the directory listing so it looks swanky. Simply scans over the crash reporter submitted directory for matching files and displays them sorted by date. Provides a delete button to remove all crash reports. Tested on all 3 main platforms. Let me know if I require an additional review for the changes in docshell, though they are pretty clear cut.
Attachment #297149 - Flags: review?
Attachment #297149 - Flags: review? → review?(mano)
According to the comment in docshell this code needs security review before being allowed to have chrome privileges in this way. The code displays an abbreviated form of filenames that are present on the local filesystem and provides a means to delete those files.
Whiteboard: [has patch]
Missed off a file in hte previous patch
Just two offhand comments on the same topic: 1) bent did some work to let toolkit apps specify their profile dir, and the crash reporter respects that. CCing him for insight. 2) Appending "Application Support" doesn't sound like the right thing to do there. We really don't have a key for that? In native code it's: "FSFindFolder(kUserDomain, kApplicationSupportFolderType, kCreateFolder, &foundRef);" Eagerly awaiting a win32 build of this to try out!
(In reply to comment #7) > Just two offhand comments on the same topic: > > 1) bent did some work to let toolkit apps specify their profile dir, and the > crash reporter respects that. CCing him for insight. I know the code you are referring to, but I looked in the actual source of the breakpad clients and it seemed to use pretty hard and fast rules for the Crash Reports directory. > 2) Appending "Application Support" doesn't sound like the right thing to do > there. We really don't have a key for that? In native code it's: > "FSFindFolder(kUserDomain, kApplicationSupportFolderType, > kCreateFolder, &foundRef);" Nothing I could find and I did dig, I guess I could just add one to the directory service provider, up to Mano.
Aha, http://lxr.mozilla.org/mozilla/source/toolkit/xre/nsXULAppAPI.h#183 You want UAppData, that already takes bent's stuff into account. (Had to backtrack that from here: http://mxr.mozilla.org/mozilla/source/toolkit/xre/nsAppRunner.cpp#2656)
Right ok so the ifdeffed parts of the code to find the crash reports directory will change to just using UAppData except in the case of Thunderbird on mac where it needs to be the mac portion from there. I'll get that updated later on this evening probably.
Yeah, I agree with ted that as long as you use UAppData everything will work just fine.
This switches to just UAppData for the majority of the time. Thunderbird however has to be special on OSX and changes its app path to ~/Library/Thunderbird, but breakpad continues to use ~/Library/Application Support/Thunderbird.
Test builds of this are up at https://build.mozilla.org/tryserver-builds/2008-01-15_06:firstname.lastname@example.org/
Just a few interface nits: 1) Can we put the "remove" button up top? With a long list of reports, it's scrolled offscreen. 2) I'd like to change the title from "Crash Reporter" to "Submitted Crash Reports" or something like that. 3) I think "ID" should be at least "Report ID" or something clearer. Pretty awesome!
I filed bug 412477 on a REST api for Socorro that could make this even more awesome.
This moves the button, changes those two strings and also removes the redundant makefile from toolkit/crashreporter/content
FORCE_DIR vs. FORCDIR, but what's it about anyway?
Some RTL support would be nice too (see places in which you've encoded right/left).
(In reply to comment #17) > FORCE_DIR vs. FORCDIR, but what's it about anyway? In general Breakpad uses the user data directory area for storing the submitted reports, but it works this out in it's own code. This normally matches except on Thunderbird on OSX. In this cast Breakpad still uses ~/Library/Application Support/Thunderbird whereas the Thunderbird data directory is in ~/Library/Thunderbird. See here where the Thunderbird adjustment is made to UAppData making it useless for us: http://mxr.mozilla.org/seamonkey/source/toolkit/xre/nsXREDirProvider.cpp#1013 And here where Breakpad actually decides on the crash reports directory and makes note of the Thunderbird inconsistency: http://mxr.mozilla.org/seamonkey/source/toolkit/crashreporter/client/crashreporter_osx.mm#548 (In reply to comment #18) > Some RTL support would be nice too (see places in which you've encoded > right/left). I'll get a new version knocked up with that now.
Fixes the FORCE_DIR inconsistency and sorts out the rtl issue I believe.
Hg is producing some interestingly screwed up patches for some reason, this one looks more sensible
From IRC: <Mano> Mossop: should be something like <body dir="..chromedir.."> <Mano> Mossop: 2. I would rather set chromedir on individual elements This fixes those comments.
Comment on attachment 298791 [details] [diff] [review] patch rev 5 r=mano
Attachment #298791 - Flags: review?(mano) → review+
Comment on attachment 298791 [details] [diff] [review] patch rev 5 Seeking approval for this, it is pretty well contained and provides a damned useful feature for testers and bug reporters.
Attachment #298791 - Flags: approval1.9?
Comment on attachment 298791 [details] [diff] [review] patch rev 5 a=beltzner for 1.9
Attachment #298791 - Flags: approval1.9? → approval1.9+
I can haz scotch? RCS file: /cvsroot/mozilla/toolkit/crashreporter/jar.mn,v done Checking in toolkit/crashreporter/jar.mn; /cvsroot/mozilla/toolkit/crashreporter/jar.mn,v <-- jar.mn initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/crashreporter/content/crashes.xhtml,v done Checking in toolkit/crashreporter/content/crashes.xhtml; /cvsroot/mozilla/toolkit/crashreporter/content/crashes.xhtml,v <-- crashes.xhtml initial revision: 1.1 done Checking in toolkit/locales/jar.mn; /cvsroot/mozilla/toolkit/locales/jar.mn,v <-- jar.mn new revision: 1.48; previous revision: 1.47 done RCS file: /cvsroot/mozilla/toolkit/locales/en-US/crashreporter/crashes.dtd,v done Checking in toolkit/locales/en-US/crashreporter/crashes.dtd; /cvsroot/mozilla/toolkit/locales/en-US/crashreporter/crashes.dtd,v <-- crashes.dtd initial revision: 1.1 done RCS file: /cvsroot/mozilla/toolkit/locales/en-US/crashreporter/crashes.properties,v done Checking in toolkit/locales/en-US/crashreporter/crashes.properties; /cvsroot/mozilla/toolkit/locales/en-US/crashreporter/crashes.properties,v <-- crashes.properties initial revision: 1.1 done Checking in docshell/base/nsAboutRedirector.cpp; /cvsroot/mozilla/docshell/base/nsAboutRedirector.cpp,v <-- nsAboutRedirector.cpp new revision: 1.31; previous revision: 1.30 done Checking in docshell/build/nsDocShellModule.cpp; /cvsroot/mozilla/docshell/build/nsDocShellModule.cpp,v <-- nsDocShellModule.cpp new revision: 1.36; previous revision: 1.35 done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [has patch]
Missed this part of the patch on checkin somehow, if someone could check it in before the nightlies it'd be neat!
I'll catch it in the morning if noone else does it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre. Also verified on Win Vista nightly. This will be a useful tool for QA for sure.
Status: RESOLVED → VERIFIED
So... I have two questions: 1) This adds the about: page in all Gecko clients, but the preference only in Firefox. Is there a reason for this? 2) There's a nice comment in the docshell code being edited that says that any new privileged about: pages need security review. Did this happen? I see nothing to that effect in the bug... 3) Docshell code changes require sr (and ideally r= from a docshell module owner, which brings me back to question 2).
I guess that's three questions, huh? ;)
(In reply to comment #31) > So... I have two questions: > > 1) This adds the about: page in all Gecko clients, but the preference only in > Firefox. Is there a reason for this? Yes, not all gecko clients will have the same breakpad server. I'm not sure of an ideal way to handle that > 2) There's a nice comment in the docshell code being edited that says that any > new privileged about: pages need security review. Did this happen? I see > nothing to that effect in the bug... Crap crap crap, I did note this in comment 5 but failed to get this done. I will in all haste badger someone for this. > 3) Docshell code changes require sr (and ideally r= from a docshell module > owner, which brings me back to question 2). I did ask in comment 4 if an additional review were required I shoul have really checked further for myself though. Thank you for spotting this. If you want this backed out then we can go that way, I'd prefer to just try to get these sorted post checkin though.
Hmm.... How about having the page make it clear what's wrong if the URI pref is missing, and either checking in changes to tbird/seamonkey to point to the right breakpad server (the same one, right?) or just filing bugs on them to get it done? I agree that there's no good way to handle random other Gecko apps needing a different URI past making it clear that they need to set the pref. And yeah, if you can set up a security review that sounds great. No need to back out or anything, as long as said review happens before the next beta. The docshell changes do look fine pending the security review. Thanks for the quick response!
(In reply to comment #34) > Hmm.... How about having the page make it clear what's wrong if the URI pref is > missing, and either checking in changes to tbird/seamonkey to point to the > right breakpad server (the same one, right?) or just filing bugs on them to get > it done? I agree that there's no good way to handle random other Gecko apps > needing a different URI past making it clear that they need to set the pref. Filed bug 414050 and submitted patches for getting the pref set in other apps. For apps without the pref, the simple option is just to display that no crash reports were found. Sounds to me like you are preferring a more explicit message like "Crash Report listing isn't configured in this application" or something similar? > And yeah, if you can set up a security review that sounds great. No need to > back out or anything, as long as said review happens before the next beta. The > docshell changes do look fine pending the security review. Reaching out on this now, if nothing else I am in MV next week which should help.
> "Crash Report listing isn't configured in this application" Yeah, or even "The x.y preference needs to be set to the correct crash report server for this application".
Ok filed bug 414054 to make that changed.
This idea completely rocks
> And yeah, if you can set up a security review that sounds great. This happened on Feb 12, 2008: http://wiki.mozilla.org/Firefox3/StatusMeetings/2008-02-12 http://wiki.mozilla.org/Breakpad/AboutCrashes/SecurityReview
Pushed a browser chrome test to m-c: http://hg.mozilla.org/mozilla-central/rev/9373d3073796
You need to log in before you can comment on or make changes to this bug.