Bug 117669

Summary: Web Inspector: Copying array or object output does not contain values
Product: WebKit Reporter: Antoine Quint <graouts>
Component: Web InspectorAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Antoine Quint 2013-06-14 18:33:47 PDT
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"
Comment 1 Antoine Quint 2013-06-14 18:34:05 PDT
<rdar://problem/13439854>
Comment 2 Antoine Quint 2013-06-14 18:38:22 PDT
Created attachment 204753 [details]
Patch
Comment 3 Timothy Hatcher 2013-06-14 19:02:39 PDT
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.
Comment 4 Antoine Quint 2013-06-14 19:08:53 PDT
(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.
Comment 5 Timothy Hatcher 2013-06-14 19:11:27 PDT
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.
Comment 6 Antoine Quint 2013-06-19 06:34:14 PDT
(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?
Comment 7 Antoine Quint 2013-06-19 07:23:29 PDT
Created attachment 205004 [details]
Patch
Comment 8 Timothy Hatcher 2013-06-19 07:25:45 PDT
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.
Comment 9 Antoine Quint 2013-06-19 09:34:11 PDT
Created attachment 205010 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2013-06-19 10:30:21 PDT
Comment on attachment 205010 [details]
Patch for landing

Clearing flags on attachment: 205010

Committed r151738: <http://trac.webkit.org/changeset/151738>
Comment 11 WebKit Commit Bot 2013-06-19 10:30:23 PDT
All reviewed patches have been landed.  Closing bug.