Copy command on cyclic object should display an error message in the console
Categories
(DevTools :: Console, defect, P3)
Tracking
(firefox59 wontfix, firefox92 fixed)
People
(Reporter: nchevobbe, Assigned: claubatista)
Details
Attachments
(3 files, 1 obsolete file)
Steps to reproduce: 1. Open the webconsole 2. Evaluate the following code : `var a = {};a.b = a; copy(a);` Expected results: An error is displayed in the console, as if I was doing `var a = {};a.b = a; JSON.stringify(a);` ("TypeError: cyclic object value[Learn More]") Actual results: There is nothing displayed in the console, and when I try to paste the result of the copy command I get "/* TypeError: cyclic object value */". This is not useful because : - I don't have immediate feedback (and I may already have switched to my editor to paste the thing I wanted to copy, and thus I have to go back to the console, and figure out what is going on) - I don't have the "Learn more" link that could tell me what this error means.
Comment 1•6 years ago
|
||
Good idea, showing an error in the console would be much better than putting this on the clipboard
Reporter | ||
Comment 2•6 years ago
|
||
So, here is where we put the error into a comment: https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/devtools/server/actors/webconsole/utils.js#587
Comment hidden (mozreview-request) |
Reporter | ||
Comment 4•6 years ago
|
||
The patch from Comment 3 is already better than the current situation, but: - The error does not say what caused it. We should have something that say: "Error while copying object: {the actual exception}". - The error is not l10n'ed - There is no "learn more" link to make sense of the error. Also, the error is not shown if you do the copy via the context menu entry (right click on object, and select "Copy object"). We should fire the same message as if you were doing `x = {}; x.y = x;JSON.stringify(x)`, only with a prefix to indicate it is a result of the copy command.
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Comment 6•4 years ago
|
||
@Nicholas can you please assign me this task, I am working on it
Reporter | ||
Comment 7•4 years ago
|
||
sure! thanks
Comment 8•4 years ago
|
||
Bug 1430022 -Added error message in copy command on cyclic object value
Comment 9•4 years ago
|
||
Comment 10•4 years ago
|
||
@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit
Comment 11•4 years ago
|
||
(In reply to Abhinav Raj from comment #10)
@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit
Please can you abandon the extra revision? If you visit it at https://phabricator.services.mozilla.com/D62046, the scroll down to the bottom. Just above the comments box, there's an Add Action option... select Abandon revision from that, then hit submit.
Comment 12•4 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #11)
(In reply to Abhinav Raj from comment #10)
@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit
Please can you abandon the extra revision? If you visit it at https://phabricator.services.mozilla.com/D62046, the scroll down to the bottom. Just above the comments box, there's an Add Action option... select Abandon revision from that, then hit submit.
Done
Comment 13•4 years ago
|
||
(In reply to Abhinav Raj from comment #12)
(In reply to Mark Banner (:standard8) from comment #11)
(In reply to Abhinav Raj from comment #10)
@Nicolas by Mistake I have opened another PR, please ignore that, I will update the original commit
Please can you abandon the extra revision? If you visit it at https://phabricator.services.mozilla.com/D62046, the scroll down to the bottom. Just above the comments box, there's an Add Action option... select Abandon revision from that, then hit submit.
Done
Hmm, I don't see any changes, it still looks like it is open. Did you hit submit after selecting abandon revision?
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Comment 15•4 years ago
|
||
Abhinav, are you still working on this?
Comment 16•4 years ago
|
||
@nchevobbe I can take a look into this bug.
Reporter | ||
Comment 17•4 years ago
|
||
Thanks Alex, the bug is now yours :)
What needs to be done:
- In devtools/server/actors/webconsole/utils.js#620 , we don't want to assign the exception to the payload, but return an object that will indicate to the client that there was an error, like we do in devtools/server/actors/webconsole/utils.js#567-570
- add a localized string like we have in devtools/client/locales/en-US/webconsole.properties#101
- add a test case for this, in the already existing test devtools/client/webconsole/test/browser/browser_jsterm_copy_command.js
it would be nice to still pass the exception message to the client, and try to display it in the error message, but let's focus on having a message displayed first :)
Reporter | ||
Updated•2 years ago
|
Assignee | ||
Comment 18•2 years ago
|
||
Hi! I'd like to work on this. :D
Reporter | ||
Comment 19•2 years ago
|
||
thanks for helping us Claudia, the bug is now yours :)
Assignee | ||
Comment 20•2 years ago
|
||
Comment 21•2 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/55ab12720df1 [devtools] Adds an error message in the copy command on a cyclic object r=nchevobbe
Comment 22•2 years ago
|
||
bugherder |
Updated•2 years ago
|
Description
•