Fix clipboard text formatting for console.trace
Categories
(DevTools :: Console, defect, P1)
Tracking
(firefox66 fixed)
| Tracking | Status | |
|---|---|---|
| firefox66 | --- | fixed |
People
(Reporter: bgrins, Assigned: nchevobbe)
References
Details
(Whiteboard: [boogaloo-mvp])
Attachments
(5 files)
STR:
1) Paste this into console:
```
function bar() {
console.trace();
}
function foo() {
bar();
}
foo();
```
2) Highlight the console.trace() output and copy it
Expected: each function name and source location are on an individual line
Actual: locations get split onto new lines:
console.trace() debugger eval code:2:3
bar debugger eval code:2:3 foo debugger eval code:5:3 <anonymous> debugger eval code:7:1 getEvalResult
resource://devtools/server/actors/webconsole/eval-with-debugger.js:134:18
exports.evalWithDebugger
resource://devtools/server/actors/webconsole/eval-with-debugger.js:105:18
evaluateJS
resource://devtools/server/actors/webconsole.js:1016:22
evaluateJSAsync
resource://devtools/server/actors/webconsole.js:954:22
onPacket
resource://devtools/server/main.js:1706:15
send/<
resource://devtools/shared/transport/local-transport.js:64:11
exports.makeInfallible/<
resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:14
exports.makeInfallible/<
resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:14
|
Reporter | |
Comment 1•6 years ago
|
||
Noticed this when pasting a trace into Bug 1485157 (I manually reformatted it for posting onto the bug)
|
Assignee | |
Comment 2•6 years ago
|
||
Interesting: when using the "Copy message" context menu entry, it's looking just fine (which is covered by this test https://searchfox.org/mozilla-central/rev/55da592d85c2baf8d8818010c41d9738c97013d2/devtools/client/webconsole/test/mochitest/browser_webconsole_context_menu_copy_entire_message.js#58-70 )
|
||
Updated•6 years ago
|
|
|
Assignee | |
Comment 3•6 years ago
|
||
Blocking on Bug 1390768 since there are in progress changes that might conflict with any fixes we do for this bug.
|
|
Assignee | |
Comment 4•6 years ago
|
||
So first, the reason we don't have the same result from selecting with the mouse and the "Copy message" context menu entry is because the context menu uses messageElement.textContent, which does not reflect what the user would have when copying.
So the first thing to do is to not use messageElement.textContent but the Selection API:
const doc = messageEl.ownerDocument;
const win = doc && doc.defaultView;
const range = doc.createRange();
range.selectNode(messageEl);
const selection = win.getSelection();
selection.addRange(range);
clipboardText = selection.toString();
selection.removeRange(range);
This will ensure that we have the same behavior in those different case. That's also something we should use in Bug 644412 as well.
Once we have that, the webconsole clipboard tests will fail. And one of the failure is around the stacktrace.
Basically the whole stacktrace is padded by blank lines:
Error: err
a foo.js:2
b foo.js:1
foo.js:3
This is because we use a <ul> to render the stacktrace component, and, according to 🔍🦊 nsXHTMLContentSerializer.cpp#560,567, this will add a new line.
I did not found a workaround yet, except turning the <ul> into a <div> with proper aria roles. This feels ugly, but if it's what it takes to get this right, I'll implement it this way.
|
|
Assignee | |
Updated•6 years ago
|
|
|
Assignee | |
Comment 5•6 years ago
|
||
|
|
Assignee | |
Comment 6•6 years ago
|
||
Depends on D17373
|
|
Assignee | |
Comment 7•6 years ago
|
||
The Frames component changed significantly on Github, which means
we need to do some adjustments in the SmarTrace component (pass
the selectable prop, change CSS rules, …).
For now, we also hide the Framework icons as we need to find a way
to properly share the underlying CSS from the debugger (See
https://github.com/devtools-html/debugger.html/issues/7782).
Depends on D17375
|
|
Assignee | |
Comment 8•6 years ago
|
||
Having the element displayed as a block cause some new lines
to be added the clipboard when doing a selection and copying.
This would also break the changes we are doing in the "Copy message"
context menu entry.
Depends on D17376
|
|
Assignee | |
Comment 9•6 years ago
|
||
The changes were made in Github to have proper markup for the
SmartTrace component. In the console, we change how the "Copy Message"
and "Export visible messages" context menu actions work so they use
the Selection API instead of element.textContent.
The Selection API reflects what would happen if a user do a text
selection, so having our actions to do the same ensure that we keep
our markup suitable for the clipboard.
Depends on D17377
|
|
Assignee | |
Comment 10•6 years ago
|
||
Test page: https://nchevobbe.github.io/demo/console-test-app.html
You can hit console.trace buttons and trace under the React section.
|
||
Comment 11•6 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d32a5f9aaec5 [release] Redesign call stack group (#7624); r=jlast. https://hg.mozilla.org/integration/autoland/rev/c688c38605f9 [release] Make the Frames component copy-friendly (#7723); r=jlast. https://hg.mozilla.org/integration/autoland/rev/2fbbe85bb992 Adapt SmartTrace component to the modified Frames component; r=bgrins. https://hg.mozilla.org/integration/autoland/rev/b546764bfc3c Don't display the location element as block; r=fvsch. https://hg.mozilla.org/integration/autoland/rev/2ed2eb2fcea8 Fix copy/pasting stacktraces in console; r=bgrins.
|
||
Comment 12•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/d32a5f9aaec5
https://hg.mozilla.org/mozilla-central/rev/c688c38605f9
https://hg.mozilla.org/mozilla-central/rev/2fbbe85bb992
https://hg.mozilla.org/mozilla-central/rev/b546764bfc3c
https://hg.mozilla.org/mozilla-central/rev/2ed2eb2fcea8
Description
•