Closed
Bug 1303067
Opened 8 years ago
Closed 8 years ago
Enable unsubmitted crash notification for more users
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: mconley, Assigned: mconley)
References
Details
Attachments
(2 files)
58 bytes,
text/x-review-board-request
|
Felipe
:
review+
gchang
:
approval-mozilla-aurora+
|
Details |
1.66 KB,
patch
|
Details | Diff | Splinter Review |
So we landed bug 1287178, which adds a "Send Always" button to the unsubmitted crash report notification. Then we landed bug 1301722, which will suppress the notification for 30 days if it's not interacted with over several days.
Are we at a point where we're comfortable turning this notification on for more users? I know that blassey is interested in getting this up to _at least_ Beta. Do we want this to go out to Release too?
The notification is activated by the browser.crashReports.unsubmittedCheck.enabled pref, set here: http://searchfox.org/mozilla-central/rev/515bca4fca5e2e8233ed30e6b6d48fcab8497dbf/browser/app/profile/firefox.js#1515 (currently only enabled for Nightly and Aurora).
Assignee | ||
Comment 1•8 years ago
|
||
The questions are:
1) Are we at a point where we're happy to expose this notification bar to more users in order to get those crash reports?
2) Which channels should get the notification?
Flags: needinfo?(shorlander)
Assignee | ||
Comment 2•8 years ago
|
||
Hey blassey,
I understand we want this on at least Beta due to some sandboxing stuff that's going to ride the trains. Is there a bug for that work this should be attached to?
Comment 3•8 years ago
|
||
redirecting to jimm. I thought we wanted this in 51, not beta
Flags: needinfo?(blassey.bugs) → needinfo?(jmathies)
Assignee | ||
Comment 4•8 years ago
|
||
Sorry, to clarify what I said in comment 2:
Currently, this notification is held back to just Nightly and Dev Edition users. I understand that we want this on when 51 reaches Beta due to some sandboxing stuff that's going to ride the trains. Is that true? Is the idea also for the notification to reach release channel users when 51 goes out on release?
Comment 5•8 years ago
|
||
Yes, I think if the UX has been sorted out we want this to ride the trains so we can start collecting data about hidden crashes on all branches.
Comment 6•8 years ago
|
||
Sandboxing isn't blocked on or in need of this reporting.
I think product has to sign off on this UI rolling out to beta/release. Personally I'm not a fan of having this out on release since it tends to show up a lot. Beta sounds like a good idea though.
Flags: needinfo?(jmathies)
Comment 7•8 years ago
|
||
(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> The questions are:
>
> 1) Are we at a point where we're happy to expose this notification bar to
> more users in order to get those crash reports?
> 2) Which channels should get the notification?
With the improvements allowing people to always send and the auto-dismiss I think this will be fine for beta.
This is kind of a band-aid fix for us to get these crash reports. I don't see this as something we would show release users.
Flags: needinfo?(shorlander)
Comment 8•8 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #6)
> Sandboxing isn't blocked on or in need of this reporting.
>
> I think product has to sign off on this UI rolling out to beta/release.
> Personally I'm not a fan of having this out on release since it tends to
> show up a lot. Beta sounds like a good idea though.
The point of Mike's work is to make it only show once assuming users will then click always or never submit.
(In reply to Stephen Horlander [:shorlander] from comment #7)
> (In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> > The questions are:
> >
> > 1) Are we at a point where we're happy to expose this notification bar to
> > more users in order to get those crash reports?
> > 2) Which channels should get the notification?
>
> With the improvements allowing people to always send and the auto-dismiss I
> think this will be fine for beta.
>
> This is kind of a band-aid fix for us to get these crash reports. I don't
> see this as something we would show release users.
band-aid implies that we think this will be fixed in some other, better way in the future, but I'm unaware of what that other work is. Can you link me to a bug?
Flags: needinfo?(shorlander)
Comment 9•8 years ago
|
||
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #8)
> band-aid implies that we think this will be fixed in some other, better way
> in the future, but I'm unaware of what that other work is. Can you link me
> to a bug?
I think we could (probably should) rethink how the crash experience works. It feels more like an experience of accumulated parts at this point rather than a planned experience.
A result of that could be a better way to present these choices that we would feel comfortable shipping to everyone. It's also possible that we would still want to segment what UI to show users based on release vs. pre-release channels.
There is no existing initiative to do that work that I am aware of though.
As it stands though I don't think we should be showing this notification bar to general release users because: 1) it highlights Firefox crashiness as a problem when it's mostly an invisible problem to users; 2) we already have an abundance of notification bars that we inundate people with; 3) I am comfortable showing this to pre-release users because the idea of "testing" is (in theory) built in, not so much with release users
Flags: needinfo?(shorlander)
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #9)
> As it stands though I don't think we should be showing this notification bar
> to general release users because: 1) it highlights Firefox crashiness as a
> problem when it's mostly an invisible problem to users; 2) we already have
> an abundance of notification bars that we inundate people with; 3) I am
> comfortable showing this to pre-release users because the idea of "testing"
> is (in theory) built in, not so much with release users
For clarity, is our beta population included when you say "pre-release" users? In other words (sorry if I'm making you repeat yourself, I'm just trying to be absolutely sure) - can we enable this notification for users on Nightly, Dev Edition and Beta? Or must it stay only on Nightly and Dev Edition?
Flags: needinfo?(shorlander)
Comment 11•8 years ago
|
||
(In reply to Mike Conley (:mconley) - (high latency on reviews and needinfos) from comment #10)
> (In reply to Stephen Horlander [:shorlander] from comment #9)
> > As it stands though I don't think we should be showing this notification bar
> > to general release users because: 1) it highlights Firefox crashiness as a
> > problem when it's mostly an invisible problem to users; 2) we already have
> > an abundance of notification bars that we inundate people with; 3) I am
> > comfortable showing this to pre-release users because the idea of "testing"
> > is (in theory) built in, not so much with release users
>
> For clarity, is our beta population included when you say "pre-release"
> users? In other words (sorry if I'm making you repeat yourself, I'm just
> trying to be absolutely sure) - can we enable this notification for users on
> Nightly, Dev Edition and Beta? Or must it stay only on Nightly and Dev
> Edition?
Yes, by "pre-release" I was referring to Beta, Nightly and Dev Edition.
Flags: needinfo?(shorlander)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 13•8 years ago
|
||
Alright. Looking at firefox.js, I don't see us turning a lot of things off just for release. We have EARLY_BETA_OR_EARLIER, which turns on a feature for early betas only, and shuts it off for later betas. I understand that's to make the later betas that we test to be as close to what users receive as possible.
So the patch above uses that same convention and will turn the unsubmitted crash report notification for early betas or earlier.
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.
https://reviewboard.mozilla.org/r/88372/#review87452
Attachment #8804372 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 15•8 years ago
|
||
Thanks felipe.
ni?'ing myself to request uplift to Aurora.
Flags: needinfo?(mconley)
Comment 16•8 years ago
|
||
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/07ae41e73a6f
Enable unsubmitted crash notification for more users. r=Felipe
Comment 17•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.
Approval Request Comment
[Feature/regressing bug #]:
Unsubmitted crash report notification.
[User impact if declined]:
Users that run early betas will not have the opportunity to submit backlogged crash reports to Mozilla.
[Describe test coverage new/current, TreeHerder]:
This feature has been enabled on Nightly and Aurora for a while, but we want to turn it on for early betas for the next uplift. UX is fine with that, now that we automatically hide the notification if the user doesn't interact with it after a number of sessions.
[Risks and why]:
Low risk. Just a pref flip that we can flip back if needs be.
[String/UUID change made/needed]:
None.
Flags: needinfo?(mconley)
Attachment #8804372 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
status-firefox51:
--- → affected
Comment 19•8 years ago
|
||
Comment on attachment 8804372 [details]
Bug 1303067 - Enable unsubmitted crash notification for more users.
Enable unsubmitted crash notification for early beta. Take it in 51 aurora.
Attachment #8804372 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I'm hitting a conflict uplifting this to aurora, could we get a rebased patch?
Flags: needinfo?(mconley)
Assignee | ||
Comment 21•8 years ago
|
||
MozReview-Commit-ID: F1B0QA8a8wX
Assignee | ||
Updated•8 years ago
|
Attachment #8806456 -
Attachment description: Enable unsubmitted crash notification for more users → Rebased for aurora
Flags: needinfo?(mconley)
Assignee | ||
Comment 22•8 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #20)
> I'm hitting a conflict uplifting this to aurora, could we get a rebased
> patch?
Thanks for letting me know. Does this one work better?
Flags: needinfo?(wkocher)
Assignee | ||
Comment 23•8 years ago
|
||
Where "this one" is attachment 8806456 [details] [diff] [review]
Comment 24•8 years ago
|
||
bugherder uplift |
Appears so, thanks.
Flags: needinfo?(wkocher)
Updated•8 years ago
|
Assignee: nobody → mconley
Comment 26•8 years ago
|
||
This change (together with bug 1282776) will make some dashboards and automated notification systems a bit unhappy twice for each cycle (at the beginning of the cycle we'll always see spikes, at the middle of the cycle we'll always see volume reductions).
Comment 27•8 years ago
|
||
how is EARLY_BETA_OR_EARLIER defined? (it looks like the mechanism is still active in 51.0b7)
Flags: needinfo?(mconley)
Assignee | ||
Comment 28•8 years ago
|
||
(In reply to [:philipp] from comment #27)
> how is EARLY_BETA_OR_EARLIER defined? (it looks like the mechanism is still
> active in 51.0b7)
Normally EARLY_BETA_OR_EARLIER is shut off around b5, but 51 has an extended beta cycle for a variety of reasons. The flag was recently unset in a commit landed earlier today (https://hg.mozilla.org/releases/mozilla-beta/rev/d946c3ef0da36339cf3f3988547f4c6aebea37ae), so the next beta (b8) should not have the mechanism enabled.
Flags: needinfo?(mconley)
Hi Mike, Brad, enabling infobar crash submissions during early Beta cycle is making it difficult for relman to compare crash rates during early Beta cycle with past Beta cycles. Is this a necessary change? I was under the impression that we would leave this on during Nightly and Aurora cycle and not in Beta cycle. Is there such a significant benefit here? Relman team would recommend disabling this for the entire Beta cycle if possible.
Flags: needinfo?(mconley)
Flags: needinfo?(blassey.bugs)
Comment 30•8 years ago
|
||
As I said at the channel meeting, there are classes of crashes that will go unreported without the info bar. I don't know why we would purposefully not collect that data.
Flags: needinfo?(blassey.bugs)
Comment 31•8 years ago
|
||
Why is it only enabled for half the beta cycle? Enabling it for the entire beta cycle would solve the issue of crash rates changing suddenly at the middle and at the beginning of the cycle.
Assignee | ||
Comment 32•8 years ago
|
||
(In reply to Marco Castelluccio [:marco] from comment #31)
> Why is it only enabled for half the beta cycle? Enabling it for the entire
> beta cycle would solve the issue of crash rates changing suddenly at the
> middle and at the beginning of the cycle.
If I recall correctly, it's because the later betas are supposed to be as close as possible to what we finally ship as release.
Flags: needinfo?(mconley)
You need to log in
before you can comment on or make changes to this bug.
Description
•