Closed
Bug 1384560
Opened 7 years ago
Closed 7 years ago
Annotate crash reports with accessibility client information
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: jimm, Assigned: jimm)
Details
(Whiteboard: aes+)
Attachments
(1 file, 4 obsolete files)
7.15 KB,
patch
|
bugzilla
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We ship oop clients to telemetry here[1], and in-proc clients here [2]. We should add this information to crash report annotations (non public) so we don't have to keep asking users what it is they run. [1] http://searchfox.org/mozilla-central/source/accessible/windows/msaa/LazyInstantiator.cpp#351 [2] http://searchfox.org/mozilla-central/source/accessible/windows/msaa/Compatibility.cpp#134
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jmathies
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8890568 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8890842 -
Attachment is obsolete: true
Assignee | ||
Comment 3•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8890845 -
Flags: review?(aklotz)
Assignee | ||
Comment 4•7 years ago
|
||
Comment on attachment 8890845 [details] [diff] [review] patch Review of attachment 8890845 [details] [diff] [review]: ----------------------------------------------------------------- I just noticed I'm including the exception handler header twice in both files, will address in a follow up.
Assignee | ||
Comment 5•7 years ago
|
||
Went ahead and updated.
Attachment #8890845 -
Attachment is obsolete: true
Attachment #8890845 -
Flags: review?(aklotz)
Attachment #8890851 -
Flags: review?(aklotz)
Comment 6•7 years ago
|
||
Comment on attachment 8890851 [details] [diff] [review] patch v.2 Review of attachment 8890851 [details] [diff] [review]: ----------------------------------------------------------------- You are going to need to fix the following preprocessor defines around LazyInstantiator::AccumulateTelemetry to make the conditional defined(MOZ_TELEMETRY_REPORTING) || defined(MOZ_CRASHREPORTER) http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.cpp#278 http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.cpp#247 http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.h#89 Then you'll want to make the call to Telemetry::ScalarSet conditional on defined(MOZ_TELEMETRY_REPORTING) only. r- because I'd like to take another look before landing.
Attachment #8890851 -
Flags: review?(aklotz) → review-
Comment 7•7 years ago
|
||
Also http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.h#154
Comment 8•7 years ago
|
||
and http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/accessible/windows/msaa/LazyInstantiator.h#14
Assignee | ||
Comment 9•7 years ago
|
||
updated per comments.
Attachment #8890851 -
Attachment is obsolete: true
Attachment #8892923 -
Flags: review?(aklotz)
Updated•7 years ago
|
Attachment #8892923 -
Flags: review?(aklotz) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 10•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb848f123a8 Annotate crash reports with accessibility client information. r=aklotz
Keywords: checkin-needed
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8fb848f123a8
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8892923 [details] [diff] [review] patch v.3 Approval Request Comment [Feature/Bug causing the regression]: useful debugging tool - changes add annotations about a11y client to crash reports. [User impact if declined]: none [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: new crash annotations [String changes made/needed]: none
Attachment #8892923 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox56:
--- → affected
Comment 13•7 years ago
|
||
Comment on attachment 8892923 [details] [diff] [review] patch v.3 Looks useful for crash-stats info, let's uplift this for beta 1.
Attachment #8892923 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/411f7b44aee0
Comment 15•7 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #12) > [Is this code covered by automated tests?]: > no > [Has the fix been verified in Nightly?]: > yes > [Needs manual test from QE? If yes, steps to reproduce]: > no Setting qe-verify- based on Jim's assessment on manual testing needs.
Updated•7 years ago
|
Flags: qe-verify-
Comment 16•7 years ago
|
||
https://hg.mozilla.org/projects/date/rev/8fb848f123a8f1b22a61bcbb57a1583f40e64b36 Bug 1384560 - Annotate crash reports with accessibility client information. r=aklotz
You need to log in
before you can comment on or make changes to this bug.
Description
•