Last Comment Bug 665196 - Change in-content plugin crash UI to submit a crash report and reload the page in one step
: Change in-content plugin crash UI to submit a crash report and reload the pag...
Status: REOPENED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 10
Assigned To: Felix Fung (:felix)
:
Mentors:
Depends on: 702982 688925 698986 716945
Blocks: 688083 697964
  Show dependency treegraph
 
Reported: 2011-06-17 15:20 PDT by Verdi [:verdi]
Modified: 2014-12-10 15:20 PST (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
ne-Step 'Submit Crash Report' and 'Reload The Page' Link (8.18 KB, patch)
2011-09-08 03:42 PDT, Felix Fung (:felix)
no flags Details | Diff | Review
Crash Screen when 'No Report Available' (69.07 KB, image/png)
2011-09-08 03:44 PDT, Felix Fung (:felix)
no flags Details
Crash Screen With Crash Reporter Disabled (66.54 KB, image/png)
2011-09-08 03:45 PDT, Felix Fung (:felix)
no flags Details
Crash Screen with New Link (66.31 KB, image/png)
2011-09-08 03:46 PDT, Felix Fung (:felix)
no flags Details
One-Step 'Submit Crash Report' and 'Reload The Page' Link (7.37 KB, patch)
2011-09-08 03:51 PDT, Felix Fung (:felix)
no flags Details | Diff | Review
Alternative Submit Crash Report Screen (87.32 KB, image/png)
2011-09-09 15:00 PDT, Felix Fung (:felix)
no flags Details
Alternative Submit Crash Report Screen With Checkbox (6.33 KB, patch)
2011-09-09 16:25 PDT, Felix Fung (:felix)
no flags Details | Diff | Review
Alternative Submit Crash Report Screen With Checkbox (6.30 KB, patch)
2011-09-12 02:19 PDT, Felix Fung (:felix)
dolske: review-
Details | Diff | Review
Submit Plugin Crash Report Screen Using Checkbox (8.83 KB, patch)
2011-09-21 00:01 PDT, Felix Fung (:felix)
no flags Details | Diff | Review
Submit Plugin Crash Report Screen Using Checkbox (Screenshot) (34.69 KB, image/png)
2011-09-21 00:02 PDT, Felix Fung (:felix)
no flags Details
Submit Plugin Crash Report Screen Using Checkbox (8.67 KB, patch)
2011-09-21 01:36 PDT, Felix Fung (:felix)
no flags Details | Diff | Review
Submit Plugin Crash Report Screen Using Checkbox (9.59 KB, patch)
2011-09-23 16:55 PDT, Felix Fung (:felix)
dolske: review-
Details | Diff | Review
Submit Plugin Crash Report Screen Using Checkbox (9.49 KB, text/plain)
2011-09-23 22:29 PDT, Felix Fung (:felix)
no flags Details
Submit Plugin Crash Report Screen Using Checkbox (9.49 KB, patch)
2011-09-23 22:31 PDT, Felix Fung (:felix)
dolske: review-
Details | Diff | Review
Submit Plugin Crash Report Screen Using Checkbox (10.04 KB, patch)
2011-10-03 05:44 PDT, Felix Fung (:felix)
dolske: review-
Details | Diff | Review
Submit Plugin Crash Report Screen Using Checkbox (9.94 KB, patch)
2011-10-18 09:38 PDT, Felix Fung (:felix)
dolske: review+
Details | Diff | Review

Description Verdi [:verdi] 2011-06-17 15:20:51 PDT
Currently when, for example, a flash video crashes the in-content plugin crash UI has a link that says, "Submit crash report." It's not until after you submit a crash report that you get a second message and a link to "reload the page to try again."

We should change the original message to something like, "Submit crash report and reload the page." If the user is going to use the in-content UI for resolving the issue we might as well do this in one step instead of two. Also it solves the immediate problem of reloading the missing content in the first step instead of the second step.
Comment 1 Ted Mielczarek [:ted.mielczarek] 2011-06-20 10:49:27 PDT
We could try to make the UI more like the existing crash reporter client, where "submit" is a checkbox, and "restart" is a button, so we could have "[x] Submit a crash report" and "[ Reload this page]", and clicking reload would submit if the box is checked, as well as reload.
Comment 2 Verdi [:verdi] 2011-07-15 16:10:36 PDT
Friendly ping. I would love to get this into Firefox 8.
Comment 3 Verdi [:verdi] 2011-08-30 06:58:04 PDT
Flash crashed this morning <http://people.mozilla.org/~mverdi/screenshots/broswing_-_blip.tv_%28since_2005%29-20110830-085354.jpg> and I was reminded that this bug is still open. If we get this in Firefox 9 we'll only be showing this less than optimal message to people for four more months.
Comment 4 Sheila Mooney 2011-08-31 17:26:28 PDT
I spoke to Dolske about this and it kind of got lost in the black hole. We have a new intern starting next week and this would be a great project for him. I'll put it on my list to followup in a couple of weeks.
Comment 5 Justin Dolske [:Dolske] 2011-08-31 17:56:59 PDT
Ahhh, NUTS. I keep dropping this on the floor. :(

So, to recap prior email on this...

0) SUMO is getting ~240K monthly hits on the support page we link to from the Firefox UI, and this is abnormally high compared to other SUMO pages (even those linked to from within Firefox). Suspicion is that this may be due to confused users wondering what to do after a plugin crashes.

1) It would be useful to add telemetry probes to help understand how many plugin crashes happen vs how many are reported (or fail to generate a report, ala bug 554451). Also add a modifier to the help links (in-content UI and notification bar) to help understand which is generating more clicks.

2) Verdi suggests that the in-content UI is confusing because it doesn't offer any immediate solution to the problem -- just links to submit a crash report and a "?" icon...

  We could improve the steps of "Submit crash report" --> *click* --> "Submitting..." --> "Crash report sent. Reload the page to try again" to something that immediately suggests a helpful action to perform. For example, "Reload page and submit crash report" --> *click* --> reload page + background submission.

3) The suggestion was made in the thread to modify the notification bar, so make better explain that the _page_ was using a plugin, even it it wasn't visible, and to change the 2 buttons to a checkbox + 1 button.

old:

  The Flash plugin crashed. _Learn More…_    (Reload)  (Submit crash report)

new: (abbreviated here for space):

  Page was using Flash plugin, it crashed. _Learn More…_ [x] Submit report (Reload)

4) Possibly do a survey to better understand why people click the help link, which might lead to other article/UI changes. (Out of scope for this bug, though)
Comment 6 Justin Dolske [:Dolske] 2011-08-31 18:00:38 PDT
Bouncing bug over to Margaret as a placeholder; let's talk about making this an early project for Felix?

I meant to note above: all these changes look reasonable to me. I'd like to review the original bug to see if there were any specific reasons we avoided any of this originally, but I think we should proceed. #3 might need a privacy review. The link tweaks in #2 would be nice to make in Aurora (Beta?) so we can make some before/after comparisons.
Comment 7 Ted Mielczarek [:ted.mielczarek] 2011-09-01 05:13:09 PDT
Sort of repeating myself from comment 1, but one of the driving design forces behind the crash reporter client UI was "make it so users can get back to what they were doing as quickly and easily as possible". Getting crash data was secondary, but we tried to make it so that it went hand in hand. That's why we made the check-box to submit crash data opt-out by default, so all users have to do is click "Restart Firefox" and they're back up and running (and we get a crash report by default).
Comment 8 :Margaret Leibovic 2011-09-01 09:15:35 PDT
(In reply to Justin Dolske [:Dolske] from comment #6)
> I meant to note above: all these changes look reasonable to me. I'd like to
> review the original bug to see if there were any specific reasons we avoided
> any of this originally, but I think we should proceed.

Could you link to that bug and/or set a dependency?
Comment 9 Felix Fung (:felix) 2011-09-08 03:42:18 PDT
Created attachment 559091 [details] [diff] [review]
ne-Step 'Submit Crash Report' and 'Reload The Page' Link

Summary of Changes:

- Made aggregate function submitReportAndReload, does as it says on the tin.
- Updated some string
- Just re-arranged some display logic so the original reload message doesn't show alongside the new message. Screenshots attached below.
Comment 10 Felix Fung (:felix) 2011-09-08 03:44:24 PDT
Created attachment 559092 [details]
Crash Screen when 'No Report Available'
Comment 11 Felix Fung (:felix) 2011-09-08 03:45:30 PDT
Created attachment 559093 [details]
Crash Screen With Crash Reporter Disabled
Comment 12 Felix Fung (:felix) 2011-09-08 03:46:24 PDT
Created attachment 559094 [details]
Crash Screen with New Link
Comment 13 Felix Fung (:felix) 2011-09-08 03:51:22 PDT
Created attachment 559095 [details] [diff] [review]
One-Step 'Submit Crash Report' and 'Reload The Page' Link

Just some style changes
Comment 14 Dão Gottwald [:dao] 2011-09-08 08:22:54 PDT
(In reply to Ted Mielczarek [:ted, :luser] from comment #7)
> Sort of repeating myself from comment 1, but one of the driving design
> forces behind the crash reporter client UI was "make it so users can get
> back to what they were doing as quickly and easily as possible". Getting
> crash data was secondary, but we tried to make it so that it went hand in
> hand. That's why we made the check-box to submit crash data opt-out by
> default, so all users have to do is click "Restart Firefox" and they're back
> up and running (and we get a crash report by default).

+1

Submitting a crash report should remain opt in. While the user can reload the page without the link, "Reload page and send crash report" makes it look like there's no choice.
Comment 15 Verdi [:verdi] 2011-09-08 09:28:36 PDT
(In reply to Dão Gottwald [:dao] from comment #14)
> 
> Submitting a crash report should remain opt in. While the user can reload
> the page without the link, "Reload page and send crash report" makes it look
> like there's no choice.

This doesn't change the current behavior, it just makes it one step instead of two. With the current set up you have no choice but to "submit crash report" or reload the page manually.
Comment 16 Dão Gottwald [:dao] 2011-09-08 11:04:01 PDT
(In reply to Verdi from comment #15)
> (In reply to Dão Gottwald [:dao] from comment #14)
> > Submitting a crash report should remain opt in. While the user can reload
> > the page without the link, "Reload page and send crash report" makes it look
> > like there's no choice.
> 
> This doesn't change the current behavior, it just makes it one step instead
> of two. With the current set up you have no choice but to "submit crash
> report" or reload the page manually.

The current UI doesn't make much sense, frankly. On top of that, the proposed change doesn't merely maintain the current behavior. The user currently doesn't feel obliged to submit data in order to reload, as we don't tell him beforehand that the option to reload will appear as a second step.
Comment 17 Verdi [:verdi] 2011-09-08 13:50:11 PDT
(In reply to Dão Gottwald [:dao] from comment #16)
> The user
> currently doesn't feel obliged to submit data in order to reload, as we
> don't tell him beforehand that the option to reload will appear as a second
> step.

I'm not sure they don't feel obliged to submit data. The only in-content options are to submit a crash report or click the question mark. 

What's also troubling to me about those two options is that about 60,000 people click the question mark each week (4M total), taking them to a SUMO article which tells them to reload the page https://support.mozilla.com/en-US/kb/Plugin%20crash%20reports That's > 2X the next most clicked in-product sumo article. I think this is because the UI doesn't give people the solution to their problem which is to reload the page.
Comment 18 Dão Gottwald [:dao] 2011-09-08 13:55:20 PDT
(In reply to Verdi from comment #17)
> the next most clicked in-product sumo article. I think this is because the
> UI doesn't give people the solution to their problem which is to reload the
> page.

Sure, I agree that the current UI is broken. There's no reason why the user should have to submit data in order to get the plugin working again.
Comment 19 Felix Fung (:felix) 2011-09-09 15:00:41 PDT
Created attachment 559587 [details]
Alternative Submit Crash Report Screen

What about a checkbox alternative?
Comment 20 Felix Fung (:felix) 2011-09-09 16:25:31 PDT
Created attachment 559614 [details] [diff] [review]
Alternative Submit Crash Report Screen With Checkbox

Patch reflecting screenshot with checkbox
Comment 21 Ted Mielczarek [:ted.mielczarek] 2011-09-10 08:45:07 PDT
Comment on attachment 559587 [details]
Alternative Submit Crash Report Screen

This looks good. We should be populating the checkbox using the default pref, I know we have code for that. (Why is the checkbox on the wrong side of the text in this screenshot?)
Comment 22 Felix Fung (:felix) 2011-09-12 02:19:28 PDT
Created attachment 559837 [details] [diff] [review]
Alternative Submit Crash Report Screen With Checkbox

Move checkbox to the correct side of label
Comment 23 Justin Dolske [:Dolske] 2011-09-20 17:24:20 PDT
First, a bit of history about this UI FTR...

The main implementation(s) occurred in bug 538910, then bug 550293, and finally bug 557661. The UX was worked out in those bugs, in person at a work week, and after meeting with Legal to discuss privacy issues.

The design was tricky to balance, because it had goals that were difficult to reconcile:

1) Out-of-process-plugins was new and also being backported to 3.6. We very much wanted to push users towards submitting crash reports to help ensure OOPP was stable and its impact was understood.

2) The UI needed to be compact and minimal. Having a plugin crash can be a bit disorienting for a user, and isn't a very good time to be asking them to make decisions. "Just fix it!"

3) Respect the user's privacy.

We actually had a early design similar to what we want now (see bug 538910 comment 14), but that started getting tricky to implement well (see bug 538910 comment 24), and so we started exploring opt-out designs as being simpler and better for #1 and #2 above. In retrospect the opt-out design hurt #3 above (leading to the changes in bug 550293 and bug 557661), and I ended up finding a way to address my my technical concern (cmt 24) for another need anyway.

tl;dr: we considered this design, moved away for other reasons, but should be OK to do it now. On to the review!
Comment 24 Justin Dolske [:Dolske] 2011-09-20 17:37:20 PDT
Comment on attachment 559095 [details] [diff] [review]
One-Step 'Submit Crash Report' and 'Reload The Page' Link

Let's go with the checkbox version.
Comment 25 Justin Dolske [:Dolske] 2011-09-20 18:03:17 PDT
Comment on attachment 559837 [details] [diff] [review]
Alternative Submit Crash Report Screen With Checkbox

Review of attachment 559837 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +7133,1 @@
>  

Now that we've got a checkbox, the checkbox value should mirror the pref... Add a pref observer, using the "mozCleverClosureHack" from a few lines up to avoid leaks.

Oh. Except this isn't a real pref, it's just an attribute on nsICrashReporter. Sigh. Let's just fire a new notification when this attribute is toggled so we can observe it here. Bug 540532 implemented the API, you should just need to add a NotifyObservers (nsIObserverService) in nsXULAppInfo::SetSubmitReports().

Let's do that in a separate bug that depends on this one.

::: toolkit/mozapps/plugins/content/pluginProblem.xml
@@ +68,5 @@
>              <html:div class="msg msgManagePlugins"><html:a class="managePluginsLink" href="">&managePlugins;</html:a></html:div>
>              <html:div class="submitStatus">
>                  <!-- links set at runtime -->
> +                <html:div class="msg msgPleaseSubmit">
> +                    <xul:checkbox label="&report.please;" class="pleaseSubmitCheckbox" />

Please use <html:input type="checkbox"> here. Shouldn't need to wrap it in a <div>, but it should be hidden upon submission.

Also, let's put this after msgReload, so that the UI looks like...

                  :(

      The Flash plugin crashed.
   _Reload_the_page_ to try again.
      [X] …and send crash report.
Comment 26 Felix Fung (:felix) 2011-09-21 00:01:07 PDT
Created attachment 561397 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Summary of Changes:
Made changes suggested by comment 25

Note: Checkbox is to the right of the text as is more usual and will appear on the left in RTL languages.
Comment 27 Felix Fung (:felix) 2011-09-21 00:02:36 PDT
Created attachment 561399 [details]
Submit Plugin Crash Report Screen Using Checkbox (Screenshot)
Comment 28 Dão Gottwald [:dao] 2011-09-21 01:01:56 PDT
(In reply to Felix Fung from comment #26)
> Note: Checkbox is to the right of the text as is more usual and will appear
> on the left in RTL languages.

I don't understand your reasoning here.

Also, when the checkbox is after the reload link, I think it should probably be checked by default. :/ I don't think we need to be religious about opt-in vs opt-out; if it's opt-out, the message just needs to be very clear.
Alternatively, as a backup plan, are we at least going to notice when the plugin crashreport rate suddenly drops on certain channels?
Comment 29 Felix Fung (:felix) 2011-09-21 01:36:46 PDT
Created attachment 561411 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Changes:
comment 28 -> My mistake about the positioning of the checkbox

As for opt-in of the checkbox, I believe that should be taken care of in bug 688083
Comment 30 Felix Fung (:felix) 2011-09-23 16:55:08 PDT
Created attachment 562194 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Additional Changes:
- Comment typo fixed
- Submit Checkbox checked based on gCrashReporter 'preference'
Comment 31 Justin Dolske [:Dolske] 2011-09-23 17:37:39 PDT
Comment on attachment 562194 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Review of attachment 562194 [details] [diff] [review]:
-----------------------------------------------------------------

Close, just a few more things that should be obvious to fix.

Looks like you've split some work off to bug 688083. We'll probably want to land both together...

::: browser/base/content/browser.js
@@ +6811,5 @@
>      BrowserOpenAddonsMgr("addons://list/plugin");
>    },
>  
> +  // When user clicks try, checks if we should also send crash report in bg
> +  retryPluginPage: function (plugin, pluginDumpID, browserDumpID) {

Nit: make this function's args be (browser, checkbox, pluginDumpID, browserDumpID).

That avoids the need to refetch browser, and may be slightly less future-cleanup for electrolysis (where this code won't be able to touch content elements).

@@ +6829,2 @@
>      // The crash reporter wants a DOM element it can append an IFRAME to,
>      // which it uses to submit a form. Let's just give it gBrowser.

Hmm. This comment seems to have become obsolete at some point. Looks like bug 548667 or something before it... Remove it while you're here? Thanks!

::: toolkit/locales/en-US/chrome/mozapps/plugins/plugins.dtd
@@ +33,5 @@
>  <!ENTITY reloadPlugin.pre                                    "">
>  <!ENTITY reloadPlugin.middle                                 "Reload the page">
>  <!ENTITY reloadPlugin.post                                   " to try again.">
>  <!-- LOCALIZATION NOTE (report.please): This and the other report.* strings should be as short as possible, ideally 2-3 words. -->
> +<!ENTITY report.please                                       "… and send crash report">

No space after the ellipsis and period at the end, I think? (paging grammar nazis…)

Also, when changing localized strings, the entity/property name has to be changed so that our localization tools let localizers know something's different. (For spelling fixes, the name can stay the same though.)

::: toolkit/mozapps/plugins/content/pluginProblem.xml
@@ +72,3 @@
>                  <!-- links set at runtime -->
> +                <html:input id="pleaseSubmitCheckbox" type="checkbox" class="msg pleaseSubmitCheckbox" />
> +                <html:label for="pleaseSubmitCheckbox" dir="&locale.dir;" class="msg pleaseSubmitLabel">&report.please;</html:label>

Was the |dir| actually needed here for proper RTL appearance? I'd sorta expect the right thing to just automagically happen...

::: toolkit/mozapps/plugins/content/pluginProblemContent.css
@@ +53,5 @@
>  
> +.submitStatus[status="please"]     .pleaseSubmitCheckbox,
> +.submitStatus[status="please"]     .pleaseSubmitLabel {
> +  display: inline;
> +}

Hmm, just put the checkbox and label into a wrapper <div>? Then you just need one rule to control both, and it can remain grouped with the other "display: block" rules right below.
Comment 32 Felix Fung (:felix) 2011-09-23 22:29:46 PDT
Created attachment 562215 [details]
Submit Plugin Crash Report Screen Using Checkbox

Made recommended changes above.
Comment 33 Felix Fung (:felix) 2011-09-23 22:31:09 PDT
Created attachment 562216 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Made additional changes requested above.
(Forgot to mark content type as patch)
Comment 34 Justin Dolske [:Dolske] 2011-10-02 15:45:58 PDT
Comment on attachment 562216 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Review of attachment 562216 [details] [diff] [review]:
-----------------------------------------------------------------

r- for still needing to change the entity name, but looks good otherwise.

::: toolkit/locales/en-US/chrome/mozapps/plugins/plugins.dtd
@@ +33,5 @@
>  <!ENTITY reloadPlugin.pre                                    "">
>  <!ENTITY reloadPlugin.middle                                 "Reload the page">
>  <!ENTITY reloadPlugin.post                                   " to try again.">
>  <!-- LOCALIZATION NOTE (report.please): This and the other report.* strings should be as short as possible, ideally 2-3 words. -->
> +<!ENTITY report.please                                       "…and send crash report">

As noted in prior review, you need to change "report.please" to some other sting to make our localization tools notice this change. "report.checkbox" or something.

::: toolkit/mozapps/plugins/content/pluginProblem.xml
@@ +72,1 @@
>                  <!-- links set at runtime -->

Tiniest Nit Evar: This comment no longer applies, since there are no <a> links below it to set. Please remove.
Comment 35 Justin Dolske [:Dolske] 2011-10-02 16:38:53 PDT
Oh, hmm, I just realized there's a couple more things. Sorry. :/

1) The msgSubmitPlease <div> (aka the checkbox+msg) should be hidden once the crash report is submitted (from any instance, via CSS). Having the UI hang around past then isn't useful.

2) As-is, I think you end up with retryPluginPage() submitting the report again if the user clicks "reload page" in a different plugin instance. That's not good. Maybe have the observer removeEventListener + hook it up to reloadPage()?
Comment 36 Felix Fung (:felix) 2011-10-03 05:44:09 PDT
Created attachment 564165 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Additional Changes:
- used ifdef #MOZ_CRASHREPORTER to determine whether link callback was reloadPage or retryPluginPage
- report.please -> report.checkbox
- cleaned up some comments
- only submit crash reports if we've not already done so
- removed status/checkbox ui after a report has been sent. ergo, there is no longer any messages showing whether a submissions has succeeded since presumably you've reloaded the page already and instances elsewhere don't care
Comment 37 Justin Dolske [:Dolske] 2011-10-17 14:46:17 PDT
Comment on attachment 564165 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Review of attachment 564165 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +6830,5 @@
> +    let submitChk =
> +      doc.getAnonymousElementByAttribute(plugin, "class", "pleaseSubmitCheckbox");
> +
> +    // Check status to make sure we haven't submitted already
> +    if (status != "success" && status != "failed" && submitChk.checked) {

This should just be | if (status == "please" && .checked)|. Otherwise you can again submit an already-submitting report.

I think you're trying to handle resubmitting a failed report, but I don't think that'll work here (we should tackle that as part of an about:crashes revamp).

::: toolkit/mozapps/plugins/content/pluginProblem.xml
@@ -71,5 @@
> -                <html:div class="msg msgPleaseSubmit"><html:a class="pleaseSubmitLink" href="">&report.please;</html:a></html:div>
> -                <html:div class="msg msgSubmitting">&report.submitting;<html:span class="throbber"> </html:span></html:div>
> -                <html:div class="msg msgSubmitted">&report.submitted;</html:div>
> -                <html:div class="msg msgNotSubmitted">&report.disabled;</html:div>
> -                <html:div class="msg msgSubmitFailed">&report.failed;</html:div>

Why remove the "submitted" and "failed" states?
Comment 38 Justin Dolske [:Dolske] 2011-10-18 09:13:31 PDT
Comment on attachment 564165 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Oops, forgot to mark this.
Comment 39 Felix Fung (:felix) 2011-10-18 09:38:48 PDT
Created attachment 567772 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Addressed above comments
Comment 40 Justin Dolske [:Dolske] 2011-10-19 14:10:27 PDT
Comment on attachment 567772 [details] [diff] [review]
Submit Plugin Crash Report Screen Using Checkbox

Review of attachment 567772 [details] [diff] [review]:
-----------------------------------------------------------------

\o/
Comment 42 :Margaret Leibovic 2011-10-27 11:33:45 PDT
https://hg.mozilla.org/mozilla-central/rev/5df0d4237670
Comment 43 Verdi [:verdi] 2011-10-31 11:57:03 PDT
(In reply to Felix Fung (:felix) from comment #29)
> Created attachment 561411 [details] [diff] [review] [diff] [details] [review]
> Submit Plugin Crash Report Screen Using Checkbox
> 
> Changes:
> comment 28 -> My mistake about the positioning of the checkbox
> 
> As for opt-in of the checkbox, I believe that should be taken care of in bug
> 688083

Sorry, I can't follow the code submissions and figure out what the final workflow is. I have a couple of questions for documentation:
 
1. What is the default behavior? If I see the in-content UI for a plugin crash and I just click the link without reading the message, will I send a crash report or not? 
2. What is the final wording of the message?
Comment 44 Felix Fung (:felix) 2011-11-04 16:40:24 PDT
Reply to comment 43:

1) As soon as the UI appears, there is also a checkbox. The checkbox is attached to the pref of whether the user wants to submit crash reports or not. I believe this is first set on install and most users should have it set one way or the other already. So the checkbox will reflect reality and whether or not a crash report will be sent.

2) In totality it'll look something like:
The Foo Plugin has crashed.
Reload the page to try again.
x] ... and send crash report.
Comment 45 Verdi [:verdi] 2011-11-04 17:00:32 PDT
(In reply to Felix Fung (:felix) from comment #44)
Beautiful! Thank you so much for fixing this!
Comment 46 Francesco Lodolo [:flod] 2012-01-25 22:27:46 PST
Aurora is supposed to be string frozen and you broke everything (toolkit=every single product we ship) a week before the move to beta. Is this right?
Comment 47 Francesco Lodolo [:flod] 2012-01-25 22:32:36 PST
(In reply to Francesco Lodolo [:flod] from comment #46)
> Aurora is supposed to be string frozen and you broke everything
> (toolkit=every single product we ship) a week before the move to beta. Is
> this right?

Sorry, Ignore this... wrong bug (I checked the revision history on central instead of aurora, shouldn't be doing this at 7am...)

Right bug is 716945 and Axel is already on that.
Comment 48 Verdi [:verdi] 2012-02-03 12:04:29 PST
It appears that there was a problem with the implementation that caused crash reports to not be sent so this is being backed out (bug 716945). The initial problem still remains then. We now have more reliable numbers on SUMO (due to new webtrends js) and we get about 120,000 visits to the linked article each week with 99% of traffic coming from the UI.

We should implement this in a way that doesn't prevent the crash reports from going through.

Note You need to log in before you can comment on or make changes to this bug.