Closed Bug 789674 Opened 7 years ago Closed 7 years ago

Update about:support to use Troubleshoot.jsm

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
about:support should be updated to use the snapshot data from Troubleshoot.jsm.

This patch also adds a "Copy raw data to clipboard" button and keeps the current copy button but renames it "Copy text to clipboard".
Attachment #659454 - Flags: review?(felipc)
Comment on attachment 659454 [details] [diff] [review]
patch

Review of attachment 659454 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/aboutSupport.js
@@ +212,3 @@
>  }
>  
> +function copyRawDataToClipboard(button) {

are the "if (button)" checks necessary? and what could fail in this function to require a try catch?

::: toolkit/locales/en-US/chrome/global/aboutSupport.dtd
@@ +59,5 @@
>  
>  <!ENTITY aboutSupport.installationHistoryTitle "Installation History">
>  <!ENTITY aboutSupport.updateHistoryTitle "Update History">
>  
> +<!ENTITY aboutSupport.copyToClipboard.label "Copy text to clipboard">

need to change entity name due to string change
Attachment #659454 - Flags: review?(felipc) → review+
Obs.: I'm assuming the addition of the new button was discussed with support and they want to have both buttons there
(In reply to :Felipe Gomes from comment #1)
> > +function copyRawDataToClipboard(button) {
> 
> are the "if (button)" checks necessary?

The function could conceivably be called without passing in a button.

> and what could fail in this function to require a try catch?

Probably nothing, but a failure shouldn't cause the button to remain disabled.

(In reply to :Felipe Gomes from comment #2)
> Obs.: I'm assuming the addition of the new button was discussed with support
> and they want to have both buttons there

I didn't discuss the buttons with them, and nobody's responded to all the recent updates to bug 554174.
https://hg.mozilla.org/mozilla-central/rev/83925e071235
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Comment on attachment 659454 [details] [diff] [review]
patch

>+        return strings.formatStringFromName(nameOrMsg, msgArray,
>+                                            msgArray.length);
There's an assertion that fires when there are no format string...
Depends on: 868793
Depends on: 1034724
Depends on: 1120161
You need to log in before you can comment on or make changes to this bug.