Closed
Bug 688083
Opened 13 years ago
Closed 13 years ago
With in-content plugin crash, send crash report option should be synced amongst windows/tabs
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 10
People
(Reporter: felix, Assigned: felix)
References
Details
Attachments
(1 file, 1 obsolete file)
7.35 KB,
patch
|
Dolske
:
review+
|
Details | Diff | Splinter Review |
When a plugin crashes (like flash) we sometimes show an in-page error box indicating that they can send crash report and/or reload the page. bug 665196 is going to make sending a crash report a checkbox option. This checkbox should be synced amongst all in-page error boxes (of which there could be multiple across windows and tabs) whenever the plugin crashes.
Assignee | ||
Comment 1•13 years ago
|
||
Summary of Changes: - Added observer for each instance via closureHack to observe changes to gCrashReporter.submitReports - Each plugin instance will send notification when send crash report checkbox is clicked
Attachment #562227 -
Flags: review?(dolske)
Comment 2•13 years ago
|
||
Comment on attachment 562227 [details] [diff] [review] Plugin Crash: Sync Submit Report Checkbox With Pref Between Instances Review of attachment 562227 [details] [diff] [review]: ----------------------------------------------------------------- Need to move the notifyObservers, but otherwise looks good. ::: browser/base/content/browser.js @@ +7079,5 @@ > + // notifies plugin crash instances to sync submit reports checkbox > + submitChk.addEventListener("click", function() { > + gCrashReporter.submitReports = this.checked; > + Services.obs.notifyObservers( > + null, "submit-reports-pref-changed", pluginDumpID); The notification should really be sent instead by the code that implements the .submitReports property... That would be SetSubmitReports() in toolkit/crashreporter/nsExceptionHandler.cpp. Look around the tree for "obsService->NotifyObservers" for how to notify from native code, it's basically the same as in JS. No need to pass along the pluginDumpID, it's fine to sync the pref globally as it's not really specific to a particular crash. @@ +7105,2 @@ > if (doPrompt) { > + let submitReportsPrefObserver = { You could fold this code into the existing observer (by checking |subject| to see how it's being called), but it's fine either way. @@ +7109,5 @@ > + observe : function(subject, topic, data) { > + let submitChk = doc.getAnonymousElementByAttribute( > + plugin, "class", "pleaseSubmitCheckbox"); > + // Ignore notifications for other crashes. > + if (data == pluginDumpID) { Don't need this check.
Attachment #562227 -
Flags: review?(dolske) → review-
Assignee | ||
Comment 3•13 years ago
|
||
Additional Changes: - Made request changes above - Didn't fold the observers. I just thought "crash-status" isn't very indicative of a notification name.
Attachment #562227 -
Attachment is obsolete: true
Attachment #564308 -
Flags: review?(dolske)
Updated•13 years ago
|
Attachment #564308 -
Flags: review?(dolske) → review+
Comment 4•13 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/fc2cf14697f8
Whiteboard: [fixed-in-fx-team]
Comment 5•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fc2cf14697f8
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 10
Comment 6•13 years ago
|
||
Comment on attachment 564308 [details] [diff] [review] Plugin Crash: Sync Submit Report Checkbox With Pref Between Instances > nsresult SetSubmitReports(bool aSubmitReports) > { >- return PrefSubmitReports(&aSubmitReports, true); >+ nsresult rv; >+ >+ nsCOMPtr<nsIObserverService> obsServ = >+ mozilla::services::GetObserverService(); >+ if (!obsServ) { >+ return NS_ERROR_FAILURE; >+ } >+ >+ rv = PrefSubmitReports(&aSubmitReports, true); Mozilla style is to declare rv here, when it's first used.
You need to log in
before you can comment on or make changes to this bug.
Description
•