Closed Bug 703564 Opened 8 years ago Closed 7 years ago

Find some way to add email or even comments for crashes on Firefox for Android

Categories

(Toolkit :: Crash Reporting, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla23
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- verified
fennec 22+ ---

People

(Reporter: kairo, Assigned: bnicholson)

References

Details

Attachments

(2 files, 2 obsolete files)

Currently, the Android crash reporter window (at least for content crashes, haven't seen anything else yet) is minimalesque, with only a checkbox for sending and close/reload buttons. We should try to find some way for people, esp. our own testers, to leave email addresses or even short comments.
Both emails and comments have proven to be very helpful when investigating crashes, and at least on tablets we should have the space to enter them.
OS: Linux → Android
Hardware: x86 → ARM
This would be a huge win for understanding the severity of stability issues. Can we track for an upcoming Fennec version?
tracking-fennec: --- → ?
Summary: Find some way to add email or even comments for crashes on Android tablets → Find some way to add email or even comments for crashes on Firefox for Android
Ian can we get an updated design for the crash reporter?
Assignee: nobody → ibarlow
tracking-fennec: ? → 19+
Keywords: uiwanted
Not tracking for any particular release, but we'd like this sooner rather than later. Ian, can we get a design?
tracking-fennec: 19+ → +
Flags: needinfo?(ibarlow)
Any news here Ian?
Attached image crash reporter mockup (obsolete) —
Changes
- Updated text
- Added inputs for comments and email address capture
- Note: I was deliberately vague when mentioning email by saying "we -may- contact you", since I'm not sure what our plan is for that. If we do indeed intend to reply to every user who includes their email, we can be more committal in that sentence.

Strings below

--------------------------------------------

Firefox has crashed

Your tabs should be listed on the Firefox Start page when you restart. 

--

You can help us fix this problem by submitting a crash report and what site you visited last. If you leave your email address we may contact you with more details. 

[x] Send Mozilla a crash report
[x] Include last visited site

Description of what happened (optional)
-----------------------------------
|                                 |
|                                 |
|                                 |
-----------------------------------

Your email (optional)
-----------------------------------
|                                 |
-----------------------------------


Your email (optional)
------------------   ---------------------
|     Close      |   |  Restart Firefox  |
------------------   ---------------------
Flags: needinfo?(ibarlow)
I'd prefer not to change the wording of UI elements from the desktop versions unless there's a really good reason to:
http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/crashreporter/crashreporter.ini

It's just more work for localizers. Application-specific things (like mentioning the Firefox Start page) are fine.
Attached image crash reporter mockup
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> I'd prefer not to change the wording of UI elements from the desktop
> versions unless there's a really good reason to:

Fair enough, here's an updated mockup. There are still a couple of string changes though. I believe our localizers may have already translated these: 

1. The second sentence of the intro is different from on desktop, since the restore behaviour is different on Android than it is on desktop.  

2. "Quit Firefox" is changed to "Close", since we don't have the same concept of quit on Android like there is on desktop.
Attachment #721190 - Attachment is obsolete: true
Setting tracking-fennec to ? to hopefully get this prioritized for a specific release (22?).
tracking-fennec: + → ?
tracking-fennec: ? → 22+
Flags: needinfo?(ibarlow)
Brad, I posted a mockup in comment 7 -- is there something else you need from UX here to proceed?
Flags: needinfo?(ibarlow)
(In reply to Ian Barlow (:ibarlow) from comment #9)
> Brad, I posted a mockup in comment 7 -- is there something else you need
> from UX here to proceed?

clearing uiwanted then
Keywords: uiwanted
I think this 22+ bug will need to be re-assigned now that we're unblocked. We're only a couple of weeks from the merge.
Assignee: ibarlow → blassey.bugs
Assignee: blassey.bugs → sriram
Sriram, need a patch here
Flags: needinfo?(sriram)
I couldn't enable crash reporter on my Mac. Hence I'm unable to see how CrashReporter would look and test it. May be someone using Linux can take this bug.
Flags: needinfo?(sriram)
I build on linux and will take a look.
Assignee: sriram → mark.finkle
(In reply to Sriram Ramasubramanian [:sriram] from comment #13)
> I couldn't enable crash reporter on my Mac. Hence I'm unable to see how
> CrashReporter would look and test it. May be someone using Linux can take
> this bug.

The Java frontend code is decoupled from breakpad (which is what can't build on OSX), you can just comment out the "ifdef MOZ_CRASHREPORTER" in mobile/android/base/Makefile.in to build the UI and launch it directly via adb.
Throwing to Brian.
Assignee: mark.finkle → bnicholson
Attached patch Improve the crash reporter (obsolete) — Splinter Review
Test build here: https://dl.dropboxusercontent.com/u/35559547/fennec-crash-reporter.apk. You can trigger the crash reporter with this build by choosing Request Desktop site from the menu.

Is there a specific name we should use for the comment/email fields? I just named them User_Comment and User_Email, so the crash data looks like this:

...
-----------------------------73595C5045D37139
Content-Disposition: form-data; name="Android_Version"

15 (REL)
-----------------------------73595C5045D37139
Content-Disposition: form-data; name="User_Comment"

hi i click request desktop site and things go boom
-----------------------------73595C5045D37139
Content-Disposition: form-data; name="User_Email"

sadpanda@gmail.com
...

Also, I added a prompt that appears when the user clicks the back button. Occasionally, I'll hit back twice -- sometimes by accident, and sometimes because an app is slow to respond. I figured we should have some sort of protection here since an accidental back click means the crash reporter screen is gone with no way to get back to it. Let me know if you want a different string here or if you think this dialog should be dropped entirely.
Attachment #739887 - Flags: review?(mark.finkle)
Attachment #739887 - Flags: feedback?(ibarlow)
They need to be named "Email" and "Comments" exactly, to match the other clients. (Sorry, that's not really documented anywhere, I had to consult the source: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/client/crashreporter_win.cpp#559 )
Comment on attachment 739887 [details] [diff] [review]
Improve the crash reporter

f+

* Make Ted's changes
* Why the string changes?
Attachment #739887 - Flags: review?(mark.finkle) → feedback+
(In reply to Mark Finkle (:mfinkle) from comment #19)
> * Why the string changes?

I changed several of the strings because of Ian's comments/mockup from comment 7 -- are those the ones you're asking about?
Status: NEW → ASSIGNED
(In reply to Brian Nicholson (:bnicholson) from comment #20)
> (In reply to Mark Finkle (:mfinkle) from comment #19)
> > * Why the string changes?
> 
> I changed several of the strings because of Ian's comments/mockup from
> comment 7 -- are those the ones you're asking about?

Yes. Thanks for the clarification.
Hi Brian, this looks great. Couple comments:

* Change title to Roboto Light, and make sure the size is 30sp -- looks a little small now

* Can we use our orange checkboxes here?

* I got stuck on the email field -- it took me a while to figure out that I had to first tap the "Allow Mozilla" checkbox before I could focus the email field. Maybe the disabled color needs to be more different from the normal color. Either way, can we also allow tapping on the disabled field, where doing so turns on the checkbox while focusing the field?

* The "Leaving this screen" dialog language is a little confusing. I might change it from

Leaving this screen means the crash will not be reported. Continue?
Don't report   |   Go back

To

Exit without sending a crash report?
Cancel    |    OK
Duplicate of this bug: 864348
(In reply to Ian Barlow (:ibarlow) from comment #22)
> * I got stuck on the email field -- it took me a while to figure out that I
> had to first tap the "Allow Mozilla" checkbox before I could focus the email
> field. Maybe the disabled color needs to be more different from the normal
> color. Either way, can we also allow tapping on the disabled field, where
> doing so turns on the checkbox while focusing the field?

On Windows at least we special-cased click events in the desktop client so that clicking on the disabled email field checks the checkbox and then focuses the field, specifically to make this interaction simpler.
Changed to use Comments/Email fields.

(In reply to Ian Barlow (:ibarlow) from comment #22)
> Hi Brian, this looks great. Couple comments:
> 
> * Change title to Roboto Light, and make sure the size is 30sp -- looks a
> little small now

Yeah, this is already 30sp. I should be able to change the font to Roboto Light, but note that the font will only be shown for 4.1 devices and higher.

> * Can we use our orange checkboxes here?

I don't think so; we're using the native Android CheckBox widget here (like we do in the prefs screen), and there's no way to customize its color without creating your own drawables.

> * I got stuck on the email field -- it took me a while to figure out that I
> had to first tap the "Allow Mozilla" checkbox before I could focus the email
> field. Maybe the disabled color needs to be more different from the normal
> color. Either way, can we also allow tapping on the disabled field, where
> doing so turns on the checkbox while focusing the field?

Ok, I darkened the disabled color to #DDD. I also made the disabled EditText clickable, but it's kind of a hack. It only works on touch presses; trying to focus/type in the EditText with a keyboard won't work.

Updated build at https://dl.dropboxusercontent.com/u/35559547/fennec-crash-reporter.apk
Attachment #739887 - Attachment is obsolete: true
Attachment #739887 - Flags: feedback?(ibarlow)
Attachment #740566 - Flags: review?(mark.finkle)
Attachment #740566 - Flags: feedback?(ibarlow)
Attachment #740566 - Flags: review?(mark.finkle) → review+
Looks great. Ship it!
Attachment #740566 - Flags: feedback?(ibarlow)
https://hg.mozilla.org/mozilla-central/rev/75648b269697
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Neat.
Status: RESOLVED → VERIFIED
Depends on: 866854
Marking as WONTFIX for Fx21/22 because of the string changes.
You need to log in before you can comment on or make changes to this bug.