518606 - Troubleshooting Information page should have better support for copy-and-paste to plaintext.

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Curtis Bartley [:cbartley]]

The Troubleshooting Information page (about:support) "Copy all to clipboard" button reformats the plain-text representation of the page to make it human readable.  There's lots of room for improvement, however.

Bonus points if we can figure out how to clean up text copied from the page with Cmd/Ctrl-A:Cmd/Ctrl-C.

Comment 2

[Drew Willcoxon :adw]

vulcain in the dupe bug 827036 suggests formatting the plain text like a spreadsheet or pretty-printed table: attachment 698318 [details]

(In reply to Curtis Bartley [:cbartley] from comment #0)
> Bonus points if we can figure out how to clean up text copied from the page
> with Cmd/Ctrl-A:Cmd/Ctrl-C.

That's bug 726605.

Comment 3

[Chris Ilias [:cilias]]

What is required to mark this bug as fixed? The descriptions seems very vague to me. The summary say "...have better support for...", and comment 0 says "There's lots of room for improvement".

This bug is probably going to stay open forever if there are no set requirements.

Comment 4

[vulcain]

@ Chris Ilias: You could prefer the description of my bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=827036

Comment 5

[Justin Dolske [:Dolske]]

As filed, this is fixed. At least, it's gone from "utterly unreadable" to "a bit ugly but decipherable (like any web page)" thanks to Drew's fine work in bug 723163. The "Copy Text To Clipboard" button on about:support makes even better output.

Comment 6

[vulcain]

@ :Dolske : If this bug is fixed by bug 723163. This both bug are not dupliacte of 827036.

Comment 7

[Drew Willcoxon :adw]

This bug is not fixed by bug 723163.  Bug 723163 only fixes the bonus-points part mentioned in comment 0.

Comment 0 makes it pretty clear that this bug is primarily about making the text produced by the "Copy text to clipboard" (née "Copy all to clipboard") function more readable.

Comment 10

[Chris Ilias [:cilias]]

More readable how?

Comment 11

[Drew Willcoxon :adw]

I think this bug is about answering that question.  But you're right that it's likely to stay open forever without more specifics, so I'll take it and try to drive it to completion, even if that means marking it WFM.

Chris, I'd like to ask SUMO contributors what they'd like to see in the formatting, since they're probably the ones most often looking at pastes from users.  Do you know if there's a forum or mailing list I could contact?

Comment 12

[Chris Ilias [:cilias]]

https://support.mozilla.org/en-US/forums/contributors

Comment 13

[Drew Willcoxon :adw]

Thanks, Chris.  Posted here: https://support.mozilla.org/en-US/forums/contributors/709167?last=52062

Comment 15

[Drew Willcoxon :adw]

In bug 599824, Helder makes this proposal:

> 1. Add less indenting in each entry;
> 2. Combine table cells into information tuples in the form "key: value";
> 3. Treat the "Profile Directory" entry differently so that either it is
> suppressed (as there's no value in outputting "Open Containing Folder" in a
> text output) or actually output the directory as text during export (for
> example, "Profile Directory: C:\Users\MyUsername\...
> 4. Remove trailing space currently added all titles ("Application Basics ",
> etc.) and entries (all of "Application Basics", like "Name ", "Version ",
> etc.)
> 5. Maybe also consider expanding/suppressing "Enabled Plugins" and "Build
> Configuration" in the simple text clipboard version, as links aren't useful
> in a text-only output (I'm not sure if there's infrastructure for this).
> 
> Proposed output would be something like:
> 
>   Application Basics
>     Name: Firefox
>     Version: 4.0b7pre
> 
> For an example take a look at bug 594025 comment 30, where I have manually
> reformatted (part of) the output automatically generated.

Bug 658186 decided that the profile path should not be included for privacy reasons, but the rest sounds good to me.  Points 2 and 5 are similar to AliceWyman's feedback here: https://support.mozilla.org/en-US/forums/contributors/709167?#post-52101

Comment 16

[Drew Willcoxon :adw]

Attachment: patch

This produces text that looks like the file I'mma attach.

Comment 17

[Drew Willcoxon :adw]

Attachment: example text

Comment 18

[Drew Willcoxon :adw]

Attachment: patch 2

Comment 19

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 751436 [details] [diff] [review]
patch 2

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

Looks good. Just a few minor fixups:

::: toolkit/content/aboutSupport.js
@@ +359,5 @@
> +    // all other elements
> +
> +    let hasText = false;
> +    for (let i = 0; i < elem.childNodes.length; i++) {
> +      let child = elem.childNodes[i];

for-of loops are *so* much nicer:
  for (let child of elem.childNodes) {

@@ +376,5 @@
> +      if (headerText) {
> +        this._startNewLine();
> +        let rule = "";
> +        for (let i = 0; i < headerText.length; i++)
> +          rule += "-";

We have String.prototype.repeat now:
  "-".repeat(headerText.length);

@@ +404,5 @@
> +    this._lines.push("");
> +  },
> +
> +  _appendText: function (text, lines) {
> +    this._currentLine += text;

Having this in a function seems overkill.

@@ +428,5 @@
> +    let hasColHeadings = Object.keys(colHeadings).length > 0;
> +    if (!hasColHeadings)
> +      tableHeadingElem = null;
> +
> +    let trs = table.querySelectorAll("table > tr, tbody tr");

Shouldn't this be "table > tr, tbody > tr" ?

@@ +472,5 @@
> +    this._startNewLine();
> +  },
> +};
> +
> +function ignoreElement(elem) {

This should be part of Serializer.

@@ +478,3 @@
>  }
>  
> +function nodeText(node) {

Ditto.

Comment 20

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b4e9af73764

(In reply to Blair McBride [:Unfocused] (Limited availability.) from comment #19)
> @@ +404,5 @@
> > +    this._lines.push("");
> > +  },
> > +
> > +  _appendText: function (text, lines) {
> > +    this._currentLine += text;
> 
> Having this in a function seems overkill.

I left it like this because I want all text updates to go through methods.  That way if we need to adjust all updates in some way or refactor this code, only one site needs to change.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7b4e9af73764