| Summary: | Web Inspector: Don't include zero-width space into a copied text from the console | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||||||
| Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Major | CC: | bburg, commit-queue, graouts, hi, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Nikita Vasilyev
2015-08-06 20:28:32 PDT
WebInspector._copy should already be doing this, I guess it isn't being called?
// Remove word break characters from the selection before putting it on the pasteboard.
var selectionString = selection.toString().removeWordBreakCharacters();
event.clipboardData.setData("text/plain", selectionString);
event.preventDefault();
Created attachment 258553 [details]
Patch
Comment on attachment 258553 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258553&action=review > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:190 > + clipboardString = clipboardString.replace(/(\s*=\s*\$\d+)$/, ""); No need for the () in the regex. Should this also remove console information? For example, entering
console.log("test");
If I copy the resulting line (which should just say "text") and paste it, I get
[Log] test
Should that [Log] be there? I have also seen source location information at the end for functions.
[Log[ is added on purpose. The location is also added. I think it makes sense to strip out the = $n parts. Created attachment 258554 [details]
Patch
Comment on attachment 258554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258554&action=review > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:190 > + if (this._message.savedResultIndex) > + clipboardString = clipboardString.replace(/\s*=\s*\$\d+$/, ""); Wait, never mind. I think $n should stay. If you use $n in a future line (like $2.foo.bar), you want to be able to look back and see where it came from. So remote this stripping. Created attachment 258555 [details]
Patch
Comment on attachment 258555 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258555&action=review > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:190 > + clipboardString = clipboardString.replace(/\s*=\s*(\$\d+)$/, " = $1"); By the way, I added this so that, as seen in Nikita's example, the " = $1" doesn't wrap to the next line. Comment on attachment 258555 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258555&action=review >> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:190 >> + clipboardString = clipboardString.replace(/\s*=\s*(\$\d+)$/, " = $1"); > > By the way, I added this so that, as seen in Nikita's example, the " = $1" doesn't wrap to the next line. Oh, I see. My bad. Comment on attachment 258555 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258555&action=review >>> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:190 >>> + clipboardString = clipboardString.replace(/\s*=\s*(\$\d+)$/, " = $1"); >> >> By the way, I added this so that, as seen in Nikita's example, the " = $1" doesn't wrap to the next line. > > Oh, I see. My bad. Oh, you were stripping before. Now you just strip linebreaks there. Nice! Comment on attachment 258555 [details] Patch Clearing flags on attachment: 258555 Committed r188184: <http://trac.webkit.org/changeset/188184> All reviewed patches have been landed. Closing bug. |