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

Status: VERIFIED FIXED

(Toolkit :: Crash Reporting, enhancement)

Description

[Michael Kraft [:morac]]

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.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+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 2

[Marien Zwart]

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

[Wayne Mery (:wsmwk)]

(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

[MrX1980]

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

Comment 5

[Robert Helmer [:rhelmer]]

Attachment: 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/

Comment 6

[Robert Helmer [:rhelmer]]

Attachment: screenshot for attachment 8753076

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

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

LGTM. Thanks, rhelmer. :)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b9eac460383
make pending crash reports more identifiable, r=mconley

Comment 9

[Phil Ringnalda (:philor)]

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)

Comment 10

[Robert Helmer [:rhelmer]]

(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

[Francesco Lodolo [:flod]]

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 12

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 13

[Robert Helmer [:rhelmer]]

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!

Comment 14

[Robert Helmer [:rhelmer]]

Comment on attachment 8753076 [details]
bug 512479 - make pending crash reports more identifiable,

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

Comment 15

[Robert Helmer [:rhelmer]]

Attachment: bug 512479 - make pending crash reports more identifiable,

Review commit: https://reviewboard.mozilla.org/r/81554/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81554/

Comment 16

[Robert Helmer [:rhelmer]]

Comment on attachment 8795514 [details]
bug 512479 - make pending crash reports more identifiable,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81554/diff/1-2/

Comment 17

[Robert Helmer [:rhelmer]]

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?

Comment 18

[Mike Conley (: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.

Comment 19

[Robert Helmer [:rhelmer]]

Comment on attachment 8795514 [details]
bug 512479 - make pending crash reports more identifiable,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81554/diff/2-3/

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28d81d95566b
make pending crash reports more identifiable, r=mconley

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28d81d95566b

Comment 22

[Nazir Ahmed Sabbir [:NaSb]]

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)

Comment 23

[Saheda Reza [:Antora]]

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]

Comment 24

[Cornel Ionce [:noni] [Hubs QA]]

Many thanks Nazir and Saheda for verifying this issue.
Closing issue based on the above comments.