Bug 1245833 - Don't send empty comments or email addresses in content crash reports. r?felipe
--- a/browser/base/content/test/general/browser_aboutTabCrashed.js
+++ b/browser/base/content/test/general/browser_aboutTabCrashed.js
@@ -120,73 +120,73 @@ function crashTabTestHelper(fieldValues,
/**
* Tests what we send with the crash report by default. By default, we do not
* send any comments, the URL of the crashing page, or the email address of
* the user.
*/
add_task(function* test_default() {
yield crashTabTestHelper({}, {
- "Comments": "",
+ "Comments": null,
"URL": "",
- "Email": "",
+ "Email": null,
});
});
/**
* Test just sending a comment.
*/
add_task(function* test_just_a_comment() {
yield crashTabTestHelper({
comments: COMMENTS,
}, {
"Comments": COMMENTS,
"URL": "",
- "Email": "",
+ "Email": null,
});
});
/**
* Test that we don't send email if emailMe is unchecked
*/
add_task(function* test_no_email() {
yield crashTabTestHelper({
email: EMAIL,
emailMe: false,
}, {
- "Comments": "",
+ "Comments": null,
"URL": "",
- "Email": "",
+ "Email": null,
});
});
/**
* Test that we can send an email address if emailMe is checked
*/
add_task(function* test_yes_email() {
yield crashTabTestHelper({
email: EMAIL,
emailMe: true,
}, {
- "Comments": "",
+ "Comments": null,
"URL": "",
"Email": EMAIL,
});
});
/**
* Test that we will send the URL of the page if includeURL is checked.
*/
add_task(function* test_send_URL() {
yield crashTabTestHelper({
includeURL: true,
}, {
- "Comments": "",
+ "Comments": null,
"URL": PAGE,
- "Email": "",
+ "Email": null,
});
});
/**
* Test that we can send comments, the email address, and the URL
*/
add_task(function* test_send_all() {
yield crashTabTestHelper({
@@ -195,8 +195,9 @@ add_task(function* test_send_all() {
email: EMAIL,
comments: COMMENTS,
}, {
"Comments": COMMENTS,
"URL": PAGE,
"Email": EMAIL,
});
});
+
--- a/browser/base/content/test/general/head.js
+++ b/browser/base/content/test/general/head.js
@@ -1144,29 +1144,34 @@ function getPropertyBagValue(bag, key) {
return null;
}
/**
* Returns a Promise that resolves once a crash report has
* been submitted. This function will also test the crash
* reports extra data to see if it matches expectedExtra.
*
- * @param expectedExtra
+ * @param expectedExtra (object)
* An Object whose key-value pairs will be compared
* against the key-value pairs in the extra data of the
* crash report. A test failure will occur if there is
* a mismatch.
*
- * Note that this will only check the values that exist
+ * If the value of the key-value pair is "null", this will
+ * be interpreted as "this key should not be included in the
+ * extra data", and will cause a test failure if it is detected
+ * in the crash report.
+ *
+ * Note that this will ignore any keys that are not included
* in expectedExtra. It's possible that the crash report
* will contain other extra information that is not
* compared against.
* @returns Promise
*/
-function promiseCrashReport(expectedExtra) {
+function promiseCrashReport(expectedExtra={}) {
return Task.spawn(function*() {
info("Starting wait on crash-report-status");
let [subject, data] =
yield TestUtils.topicObserved("crash-report-status", (subject, data) => {
return data == "success";
});
info("Topic observed!");
@@ -1195,14 +1200,18 @@ function promiseCrashReport(expectedExtr
}
info("Iterating crash report extra keys");
let enumerator = extra.enumerator;
while (enumerator.hasMoreElements()) {
let key = enumerator.getNext().QueryInterface(Ci.nsIProperty).name;
let value = extra.getPropertyAsAString(key);
if (key in expectedExtra) {
- is(value, expectedExtra[key],
- `Crash report had the right extra value for ${key}`);
+ if (expectedExtra[key] == null) {
+ ok(false, `Got unexpected key ${key} with value ${value}`);
+ } else {
+ is(value, expectedExtra[key],
+ `Crash report had the right extra value for ${key}`);
+ }
}
}
});
}
--- a/browser/modules/ContentCrashHandlers.jsm
+++ b/browser/modules/ContentCrashHandlers.jsm
@@ -158,23 +158,41 @@ this.TabCrashHandler = {
let {
includeURL,
comments,
email,
emailMe,
URL,
} = message.data;
+ let extraExtraKeyVals = {
+ "Comments": comments,
+ "Email": email,
+ "URL": URL,
+ };
+
+ // For the entries in extraExtraKeyVals, we only want to submit the
+ // extra data values where they are not the empty string.
+ for (let key in extraExtraKeyVals) {
+ let val = extraExtraKeyVals[key].trim();
+ if (!val) {
+ delete extraExtraKeyVals[key];
+ }
+ }
+
+ // URL is special, since it's already been written to extra data by
+ // default. In order to make sure we don't send it, we overwrite it
+ // with the empty string.
+ if (!includeURL) {
+ extraExtraKeyVals["URL"] = "";
+ }
+
CrashSubmit.submit(dumpID, {
recordSubmission: true,
- extraExtraKeyVals: {
- Comments: comments,
- Email: email,
- URL: URL,
- },
+ extraExtraKeyVals,
}).then(null, Cu.reportError);
this.prefs.setBoolPref("sendReport", true);
this.prefs.setBoolPref("includeURL", includeURL);
this.prefs.setBoolPref("emailMe", emailMe);
if (emailMe) {
this.prefs.setCharPref("email", email);
} else {