Closed Bug 575266 Opened 9 years ago Closed 9 years ago

Mozilla Crash Reporter should not display all options but "Close Window"

Categories

(Toolkit :: Crash Reporting, defect, trivial)

All
Windows 7
defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla2.0b12

People

(Reporter: luke_s_burrows, Assigned: rstrong)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6

Since Mozilla Crash Reporter cannot be run manually, it should not display other options like "Pin to Taskbar" except "Close Window".

Reproducible: Always

Steps to Reproduce:
1.Crash Firefox
2.The Mozilla Crash Reporter icon should appear
3.Right click on the icon
Actual Results:  
It displays "Pin to Taskbar", "crashreporter.exe" and "Close Window".

Expected Results:  
It should only display "Close Window"
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Priority: -- → P1
Hardware: x86 → All
OS: Windows 7 → All
OS: All → Windows 7
Severity: enhancement → blocker
This bug will block the release of Firefox 4 Beta 2 and Firefox 3.6.8 and will not affect Firefox 3.5 as it's approaching the End of Life scheduled for August 2010 and Firefox 3 as it is no longer supported.
If Firefox for mobile uses 1.9.2.8 for 1.1, then it will be reserved for the 1.9.2.9 branch for Firefox 3.6.
Severity: blocker → normal
Component: General → Breakpad Integration
Priority: P1 → --
Product: Firefox → Toolkit
QA Contact: general → breakpad.integration
Version: unspecified → Trunk
Severity: normal → blocker
Priority: -- → P1
Uh, why exactly should it block those releases? Seems like a minor cosmetic thing to me.
I believe the status should be New.
Firefox 3.6.8 and Firefox 4 Beta 2 can't be shipped unless it is WONTFIX, FIXED or INVALID, because "Pin to Taskbar and "crashreporter.exe" can't be used, except "Close Window".
We ship every release with plenty of bugs. We can't fix all bugs, we don't have infinite time. This very well may be a valid bug, but that doesn't mean that it's going to block either of those releases.

Does clicking "Pin to Taskbar" cause some terrible thing to happen?
The Mozilla Crash Reporter cannot be pinned to the taskbar, because it can't be run manually, but "Pin to Taskbar" does not cause a terrible thing to happen, but it should not show with "crashreporter.exe" except "Close Window" anyway. Status should be NEW and must be fixed right away, because Firefox 4 Beta 2 and Firefox 3.6.8 are blocked.
note: if / when this bug is fixed it would be a good thing to fix bug 575296 at the same time... also noted this over in bug 575296.

I also agree with Ted's evaluation of the importance of this bug
So I agree this bug should be NEW and fixed right away, because I think fixing
bug 575296 at the same time in time for Firefox 3.6.8 and Firefox 4 Beta 2
would be a great idea.
blocking1.9.2: ? → -
blocking2.0: ? → -
blocking1.9.2: - → ?
blocking2.0: - → ?
Please do not reset blocking flags after someone has set them. We do not block releases on non-serious bugs.
Severity: blocker → trivial
blocking2.0: ? → -
Severity: trivial → critical
blocking2.0: - → ?
This Bug and Bug 575296 are serious!
I believe:
http://msdn.microsoft.com/en-us/library/dd378459(VS.85).aspx#host

will work. -- I'm currently getting someone to test this.

Luke: please read https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

Please note that while you are the reporter of a bug, that does not make the bug yours. Do not mess with flags once they are minus'd.

If you continue to abuse this system, your account will be terminated. This is your only warning.
Severity: critical → trivial
blocking1.9.2: ? → ---
blocking2.0: ? → ---
Priority: P1 → --
Attached patch written w/o a compiler (obsolete) — Splinter Review
in theory there's a way to use some property store stuff, but to do that one has to properly typedef things, getprocaddress, catch each window as it's created, properly get the next thing and set it, and then properly release the resources when the windows are torn down.

note that this code hasn't met a compiler. But I did test the registry keys w/ a w7 vm.
Assignee: nobody → timeless
Attachment #454725 - Flags: review?(ted.mielczarek)
Comment on attachment 454725 [details] [diff] [review]
written w/o a compiler

I don't really know enough to review this, I think. Rob: would you like to take a crack?
Attachment #454725 - Flags: review?(ted.mielczarek) → review?(robert.bugzilla)
Comment on attachment 454725 [details] [diff] [review]
written w/o a compiler

If we are going to go with the registry method I would prefer doing this in the installer as well as removing the registry entries on uninstall.
Attachment #454725 - Flags: review?(robert.bugzilla) → review-
note: I wouldn't be adverse to the patch if it also cleaned up after itself after it is done.
rs: would using REG_OPTION_VOLATILE be satisfactory?
That might very well work though I vaguely recall running into weirdness when using it in the past. I'll try to test it over the weekend as long as I can clear my blocker list
Sorry it took so long... using REG_OPTION_VOLATILE should do the trick. After thinking about doing this with the installer I've decided that would be a PITA since there is no way to figure out when it is safe to remove the entry since there can be multiple crashreporter.exe's on the same system.
Comment on attachment 510225 [details] [diff] [review]
something like this?

this looks right, i hope you've tested it :)
Attachment #510225 - Flags: feedback?(timeless) → feedback+
Attached patch patch rev3Splinter Review
Stealing from timeless since I don't think he will mind.

Ted, I already have r+ in Bug 575296 for essentially the same patch.
Assignee: timeless → robert.bugzilla
Attachment #454725 - Attachment is obsolete: true
Attachment #510225 - Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #510371 - Flags: review?(ted.mielczarek)
Forgot to mention that I have tested it
Comment on attachment 510371 [details] [diff] [review]
patch rev3

rs=me then.
Attachment #510371 - Flags: review?(ted.mielczarek) → review+
Attachment #510371 - Flags: approval2.0? → approval2.0+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/36c70dd6a529

I personally don't think it is all that valuable to have a litmus test for this but I'll leave that up to QA to decide.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Flags: in-litmus?
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
This is such a minor issue that I don't see the value of having a litmus test
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.