Closed Bug 1298225 Opened 3 years ago Closed 3 years ago
copy/paste formatting for stack traces should be delimited between lines
33.20 KB, image/png
58 bytes, text/x-review-board-request
1.36 KB, patch
|Details | Diff | Splinter Review|
Step to reproduce. 1. Go to a site with errors. 2. Open the Console 3. Select and Copy a text with an error 4. Paste elsewhere See screenshot, selected the text with the error and its details. Copied here: TypeError: this.style.layers is undefined[Learn More]script.js:39:91F_E2http://govinfo.library.unt.edu/nssg/script.js:39:91F_OMhttp://govinfo.library.unt.edu/nssg/script.js:637:32F_doLoadedhttp://govinfo.library.unt.edu/nssg/:36:2<anonymous>http://govinfo.library.unt.edu/nssg/script.js:676:1 Instead of: TypeError: this.style.layers is undefined [Learn More] script.js :39:91 F_E2 http://govinfo.library.unt.edu/nssg/script.js :39:91 F_OM http://govinfo.library.unt.edu/nssg/script.js :637:32 F_doLoaded http://govinfo.library.unt.edu/nssg/ :36:2 <anonymous> http://govinfo.library.unt.edu/nssg/script.js :676:1
Another test page: https://bgrins.github.io/devtools-demos/console/errors.html. Click the 'generate deep stack error' button.
This clipboard output for stack traces (and console output in general) has never been great but it's been put all in one line by the shared stack trace component in Bug 1281732
Priority: -- → P2
Summary: copy/paste the console text and be readable → copy/paste formatting for stack traces should be delimited between lines
Just stashing a WIP that at least splits the output into lines. Still some issues: Error: hi thereerrors.html:81:26 hhttps://bgrins.github.io/devtools-demos/console/errors.html:81:26 ghttps://bgrins.github.io/devtools-demos/console/errors.html:80:20 fhttps://bgrins.github.io/devtools-demos/console/errors.html:79:20 ehttps://bgrins.github.io/devtools-demos/console/errors.html:78:20 dhttps://bgrins.github.io/devtools-demos/console/errors.html:77:20 chttps://bgrins.github.io/devtools-demos/console/errors.html:76:20 bhttps://bgrins.github.io/devtools-demos/console/errors.html:75:20 ahttps://bgrins.github.io/devtools-demos/console/errors.html:74:20 <anonymous>https://bgrins.github.io/devtools-demos/console/errors.html:83:5
Oh, now I see why removing the silly whitespaces and newlines from the markup in bug 1281732 wasn't a good idea. The copy&paste would deserve a mochitest, as it's an important feature that's not very visible and easy to break.
(In reply to Jarda Snajdr [:jsnajdr] from comment #4) > Oh, now I see why removing the silly whitespaces and newlines from the > markup in bug 1281732 wasn't a good idea. The copy&paste would deserve a > mochitest, as it's an important feature that's not very visible and easy to > break. Agreed about adding a simple mochitest for this. Do you happen to have some time to work on this bug?
(In reply to Brian Grinstead [:bgrins] from comment #5) > Agreed about adding a simple mochitest for this. Do you happen to have some > time to work on this bug? Yes, I think it's an important regression worth fixing quickly and uplifting.
Assignee: nobody → jsnajdr
Attached a patch with a test: I wanted to avoid putting a newline at the end of a Frame component's markup - that component is used in many places where it's not on a separate line (memory and performance tools), so I didn't want to break that. Instead, I only added whitespace between the "function name", "source link" and "host" fields in the Frame. The newlines are added to the StackTrace component, at the end of every line, together with a TAB character at the beginning. I also fixed spaces and newlines in the console output rendering - put a proper spacing between the timestamp, the message, the repeat number and the location, and a newline at the end of the message. I refactored (Task.js, ContentTask) and expanded the existing browser_console_copy_entire_message_context_menu.js test.
Comment on attachment 8786302 [details] Bug 1298225 - Format clipboard text of console stack traces into multiple lines https://reviewboard.mozilla.org/r/75276/#review73642 This looks much better, thanks! I filed https://github.com/devtools-html/gecko-dev/issues/252 for the new frontend
Attachment #8786302 - Flags: review?(bgrinstead) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/2c986272197f Format clipboard text of console stack traces into multiple lines r=bgrins
Backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=2865799&repo=autoland https://hg.mozilla.org/integration/autoland/rev/ff74f7a0d29c
Oh, I forgot to update the devtools/client/shared/components/test/mochitest/test_stack-trace.html test. My try runs didn't catch it because I ran only the "dt*" suites and this one is in "oth". Fixed, will land again if try is green.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/d29b950695f4 Format clipboard text of console stack traces into multiple lines r=bgrins
Do we need to uplift this to Beta50 as well?
(In reply to Ryan VanderMeulen [:RyanVM] from comment #18) > Do we need to uplift this to Beta50 as well? Yes please, it's a simple bugfix well covered by tests. I'm not sure if the patch will apply cleanly on the beta50 version - I'll prepare a rebased patch if it doesn't.
Please request approval on the patch then :)
Comment on attachment 8786302 [details] Bug 1298225 - Format clipboard text of console stack traces into multiple lines Approval Request Comment [Feature/regressing bug #]: 1281732 [User impact if declined]: Copying a console stack trace to clipboard won't format the text into multiple lines - all frames will be on one line [Describe test coverage new/current, TreeHerder]: Added new mochitest for the StackTrace component [Risks and why]: Low: changes how HTML markup is rendered by adding whitespace at a few places. Covered by tests [String/UUID change made/needed]: none
Attachment #8786302 - Flags: approval-mozilla-beta?
The test patch depends on earlier changes in bug 1297329 - it modifies a test added there. Going to request beta aprroval for bug 1297329, too.
See Also: → 1297329
Comment on attachment 8786302 [details] Bug 1298225 - Format clipboard text of console stack traces into multiple lines Console stack trace as a single line in clipboard makes it far less readable, stabilized on 51 for a month, Beta50+
Attachment #8786302 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Backed out from beta in https://hg.mozilla.org/releases/mozilla-beta/rev/5247dff49fb7 for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=1696316&repo=mozilla-beta
Attachment #8785098 - Attachment is obsolete: true
The tests are failing because function waitForClipboardPromise is missing in devtools/client/framework/test/shared-head.js. This function was added in bug 1269102 (part 1). Uplifting that five-part patch is too hard and risky, so I extracted just the needed part and attaching as a separate patch here. Merging the full patch later should be without problems - merge will detect that a part of the changes is already there. I ran the affected tests locally and they're passing.
https://hg.mozilla.org/releases/mozilla-beta/rev/ed69e6af42aab15b1bdf09d3705d2bd23fdc850d not sure how/if this affect the firefox 50 status
You need to log in before you can comment on or make changes to this bug.