Bug 117669 – Web Inspector: Copying array or object output does not contain values

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Antoine Quint]

Copying a result which was an array produces "< Array [size]" instead of the values, which is what I really wanted.

* STEPS TO REPRODUCE
1. js> ["one","two"]
2. Copy the resulting output row
3. Paste it somewhere
  => "< Array[2]" instead of the values "one" and "two"

[Antoine Quint]

<rdar://problem/13439854>

[Antoine Quint]

Created attachment 204753 [details]
Patch

[Timothy Hatcher]

Comment on attachment 204753 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204753&action=review

> Source/WebInspectorUI/UserInterface/ConsoleCommandResult.js:50
> +    get clipboarPrefixString ()

Typo.

> Source/WebInspectorUI/UserInterface/ConsoleMessageImpl.js:659
> +    get clipboarPrefixString ()

Typo.

> Source/WebInspectorUI/UserInterface/LogContentView.js:267
> +        var usePrefix = messages.length > 1;

I explicitly removed this logic before. I think the prefix is always useful for row selection and Commad-S. We now support text range selection if you don't want the prefix.

[Antoine Quint]

(In reply to comment #3)
> (From update of attachment 204753 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204753&action=review
> 
> > Source/WebInspectorUI/UserInterface/ConsoleCommandResult.js:50
> > +    get clipboarPrefixString ()
> 
> Typo.
> 
> > Source/WebInspectorUI/UserInterface/ConsoleMessageImpl.js:659
> > +    get clipboarPrefixString ()
> 
> Typo.

Heh, at least it's consistent.
 
> > Source/WebInspectorUI/UserInterface/LogContentView.js:267
> > +        var usePrefix = messages.length > 1;
> 
> I explicitly removed this logic before. I think the prefix is always useful for row selection and Commad-S. We now support text range selection if you don't want the prefix.

Maybe we should only drop the prefix for a single command result or prompt. I remember Joe was annoyed by it.

[Timothy Hatcher]

Comment on attachment 204753 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204753&action=review

>>> Source/WebInspectorUI/UserInterface/LogContentView.js:267
>>> +        var usePrefix = messages.length > 1;
>> 
>> I explicitly removed this logic before. I think the prefix is always useful for row selection and Commad-S. We now support text range selection if you don't want the prefix.
> 
> Maybe we should only drop the prefix for a single command result or prompt. I remember Joe was annoyed by it.

That would be fine for copy, but Command-S should always include all prefixes.

[Antoine Quint]

(In reply to comment #5)
> (From update of attachment 204753 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204753&action=review
> 
> >>> Source/WebInspectorUI/UserInterface/LogContentView.js:267
> >>> +        var usePrefix = messages.length > 1;
> >> 
> >> I explicitly removed this logic before. I think the prefix is always useful for row selection and Commad-S. We now support text range selection if you don't want the prefix.
> > 
> > Maybe we should only drop the prefix for a single command result or prompt. I remember Joe was annoyed by it.
> 
> That would be fine for copy, but Command-S should always include all prefixes.

Actually, upon further testing, the patch, as-is, does the right thing when saving the Console log via Cmd+S: all prefixes show up in the log. Could you re-review taking this into account?

[Antoine Quint]

Created attachment 205004 [details]
Patch

[Timothy Hatcher]

Comment on attachment 205004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205004&action=review

> Source/WebInspectorUI/UserInterface/ConsoleCommandResult.js:37
> +    this.enforcesClipboardPrefixString = false;

This could go on the prototype.

> Source/WebInspectorUI/UserInterface/ConsoleMessageImpl.js:41
> +    this.enforcesClipboardPrefixString = true;

Ditto.

[Antoine Quint]

Created attachment 205010 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 205010 [details]
Patch for landing

Clearing flags on attachment: 205010

Committed r151738: <http://trac.webkit.org/changeset/151738>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.