Pending crash reports should be identifiable on the "Submitted Crash Reports" page

VERIFIED FIXED in Firefox 52

Status

()

Toolkit
Crash Reporting
--
enhancement
VERIFIED FIXED
8 years ago
7 months ago

People

(Reporter: morac, Assigned: rhelmer)

Tracking

(Depends on: 2 bugs)

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
The fix to bug 378528 made it so that pending reports can now be resubmitted in case they failed to submit for whatever reason, but currently it is impossible to determine whether a report is pending or not simply by looking at the about:crashes page.

Currently the pending reports are intermixed with the submitted reports in the "Submitted Crash Reports" list on the about:crashes page.  Pending reports even have a submitted time, even though they haven't been submitted.  There should be a way of differentiating between pending reports and submitted reports.

Some suggestions:

1. Move pending reports from the "Submitted Crash Reports" list to a new "Pending Crash Reports" list which would display above the "Submitted Crash Reports" list if there are any pending crash reports.

2. If that's too difficult, change the "Submitted Crash Reports" list to "Crash Reports" and then leave the submitted time blank for pending crash reports.  That or add a " (pending)" to the id.
I would prefer one list for all crashes. That way you have continuous list of crashes. We could add another column which lists the crash date/time. So we can separate the crash time and the submitted time. For pending reports we simply don't list the submitted time.

Comment 2

8 years ago
From a support perspective it would be useful if (even if there is one list) submitted crash reports do not have a "report ID" or have one that can be told apart from an actual submitted report ID by the untrained eye (the eye of a bug reporter, not a triager or support helper).

We frequently ask people to go to about:crashes and paste over the ids or links to some recent crash reports (see https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report or the firebot "cr" factoid for example). Not having that id in there would mean we do not have to update those instructions to say something like "paste over the ids or links, but only if there is a valid "submitted" date". Just having the "submitted" date be the date of the submit attempt or the date of the crash does not matter here, but having that ID in there does.

Comment 3

8 years ago
(In reply to comment #1)
> I would prefer one list for all crashes. That way you have continuous list of
> crashes. We could add another column which lists the crash date/time. So we can
> separate the crash time and the submitted time. For pending reports we simply
> don't list the submitted time.

Comment 4

6 years ago
It is very important, because for example Adobe like to have the .dmp/.extra files.
Without the reference between pending-id and crash-id we cannot send the right files to the bug-id's.

Please add this (if possible) to the next release (Firefox 14, ...):
...\Mozilla\Firefox\Crash Reports\submitted\bp-....txt
and
crash-stats.mozilla.com/report/index/...

thanks
See Also: → bug 516455
(Assignee)

Comment 5

2 years ago
Created attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

Review commit: https://reviewboard.mozilla.org/r/52986/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52986/
Attachment #8753076 - Flags: review?(ted)
(Assignee)

Updated

2 years ago
Assignee: nobody → rhelmer
Status: NEW → ASSIGNED
(Assignee)

Comment 6

2 years ago
Created attachment 8753082 [details]
screenshot for attachment 8753076 [details]
Attachment #8753076 - Flags: review?(ted) → review?(mconley)
Comment on attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

https://reviewboard.mozilla.org/r/52986/#review79696

LGTM. Thanks, rhelmer. :)
Attachment #8753076 - Flags: review?(mconley) → review+
(Assignee)

Updated

a year ago
Attachment #8753076 - Attachment description: MozReview Request: bug 512479 - make pending crash reports more identifiable, r?ted → bug 512479 - make pending crash reports more identifiable,

Comment 8

a year ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7b9eac460383
make pending crash reports more identifiable, r=mconley
Backed out in https://hg.mozilla.org/integration/autoland/rev/7b050ca8ec64 for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4031253&repo=autoland (remains to be seen whether they are intermittent-but-too-frequent or permafailing)
(Assignee)

Comment 10

a year ago
(In reply to Phil Ringnalda (:philor) from comment #9)
> Backed out in https://hg.mozilla.org/integration/autoland/rev/7b050ca8ec64
> for test failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=4031253&repo=autoland
> (remains to be seen whether they are intermittent-but-too-frequent or
> permafailing)

This looks likely to be real, I didn't run that test first - I'll fix and re-land, thanks!

Comment 11

a year ago
mozreview-review
Comment on attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

https://reviewboard.mozilla.org/r/52986/#review79906

::: toolkit/locales/en-US/crashreporter/crashes.dtd:5
(Diff revision 1)
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
> -<!ENTITY crashes.title              "Submitted Crash Reports">
> +<!ENTITY crashes.title              "Crash Reports">

You need to change this string ID
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings
Comment on attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

https://reviewboard.mozilla.org/r/52986/#review79906

> You need to change this string ID
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Bah - I should have caught that. :/ Good eye, flod.
(Assignee)

Comment 13

a year ago
Hm I don't seem to be able to update this review anymore - I am going to close it and open a new one. Sorry for the spam!
(Assignee)

Comment 14

a year ago
mozreview-review
Comment on attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

https://reviewboard.mozilla.org/r/52986/#review80102
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8753076 - Attachment is obsolete: true
Comment hidden (mozreview-request)
(Assignee)

Comment 17

a year ago
OK got a good try run, mostly:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f1a9b702257&selectedJob=28185801

I don't think the win64 build failure could be caused by this patch...

mconley, mind taking a last quick look to make sure I didn't miss anything, in the diff or in that try run?
Flags: needinfo?(mconley)
Comment on attachment 8795514 [details]
bug 512479 - make pending crash reports more identifiable,

https://reviewboard.mozilla.org/r/81554/#review80732

Thanks! Checked the strings too, and the right ones have had their keys renamed.

::: toolkit/crashreporter/content/crashes.js:102
(Diff revision 2)
>      }
>      else {
>        link.setAttribute("href", reportURL + reports[i].id);
>      }
>      link.setAttribute("id", reports[i].id);
> +    link.setAttribute("class", "crashReport");

I'd prefer to use classList instead here, if possible:

```JavaScript
link.classList.add("crashReport");
```

::: toolkit/crashreporter/test/browser/browser_aboutCrashes.js:16
(Diff revision 2)
>  
>    yield BrowserTestUtils.withNewTab({ gBrowser, url: "about:crashes" }, function (browser) {
>      info("about:crashes loaded");
>      return ContentTask.spawn(browser, crashes, function (crashes) {
>        let doc = content.document;
> -      let crashlinks = doc.getElementById("tbody").getElementsByTagName("a");
> +      let crashlinks = doc.getElementById("submitted").getElementsByTagName("a");

Alternatively, 

```JavaScript
let crashlinks = doc.getElementById("submitted").querySelectorAll(".crashReport");
```

But really either is fine.
Attachment #8795514 - Flags: review?(mconley) → review+
Flags: needinfo?(mconley)
Comment hidden (mozreview-request)

Comment 20

a year ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/28d81d95566b
make pending crash reports more identifiable, r=mconley

Comment 21

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/28d81d95566b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1318270
Depends on: 1318274
Flags: qe-verify+
Successfully managed to reproduce this bug using version 3.6a2pre on Ubuntu 16.04.2 x64 by the following Comment 0's instruction!

This issue is now Verified as Fixed on Latest Firefox Beta 52.0b8 (64-bit)

Build ID: 20170220070057
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
OS: Linux 4.8.0-36-generic; Ubuntu 16.04.2 (64 Bit)
QA Whiteboard: [bugday-20170222]

Comment 23

10 months ago
I have reproduced this on Firefox Nightly according to (2009-08-25)

Fixing bug is verified on Latest Firefox Beta -- Build ID: (20170220070057), User Agent: Mozilla/5.0 (Windows NT 10.0; rv:52.0) Gecko/20100101 Firefox/52.0

Tested OS-- Windows10 32bit
[bugday-20170222]
Many thanks Nazir and Saheda for verifying this issue.
Closing issue based on the above comments.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.