Closed
Bug 1099071
Opened 10 years ago
Closed 8 years ago
Throwed empty string are not displayed in console
Categories
(DevTools :: Console, defect)
Tracking
(firefox47 verified, firefox48 verified)
VERIFIED
FIXED
Firefox 47
People
(Reporter: michalwadas, Assigned: ajkerrigan)
Details
Attachments
(1 file, 1 obsolete file)
4.01 KB,
patch
|
bgrins
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:33.0) Gecko/20100101 Firefox/33.0 Build ID: 20141013200110 Steps to reproduce: I typed in console: (function(){ throw ''; })(); Actual results: Nothing was displayed. Expected results: Some information should be displayed.
Reporter | ||
Updated•10 years ago
|
Component: Untriaged → Developer Tools: Console
Reporter | ||
Comment 1•9 years ago
|
||
Similar problems with empty-string .toString (function(){ throw Object.create(null) })(); (function(){ throw {toString: null, valueOf: null}; })();
Comment 2•8 years ago
|
||
Confirmed. The server is returning exceptionMessage = "" and exception = "". For an evaluation that doesn't throw it returns exception = "" and no exceptionMessage, so maybe that's enough to go off of in the frontend. The exceptionMessage is passed in here: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/webconsole.js#3166. Then looks like a bad falsy check here causes it to not show: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/console-output.js#1352.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ajkerrigan
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•8 years ago
|
||
The main change here is to the falsy check in console-output.js, but that didn't feel like a complete fix. Throwing an empty string would then correctly show an error line in the console, but the line would be blank. As a possible alternative, I made a minor tweak to webconsole.js so that it constructs an Error object in the case of a thrown string (the behavior for thrown -objects- is unchanged). Here are the changes to error output: Thrown String --> Console Error Output '' --> Error ' ' --> Error: 'test' --> Error: test Before submitting this patch I peeked around at some other dev tooling to see how these cases were handled out of curiosity. The behavior is different everywhere, but this change seems to match up with how Firebug handles thrown strings.
Attachment #8721160 -
Flags: review?(bgrinstead)
Comment 4•8 years ago
|
||
Comment on attachment 8721160 [details] [diff] [review] patch v1 Review of attachment 8721160 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! Can you add a few test cases for this to https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/test/browser_webconsole_jsterm.js? ::: devtools/client/webconsole/webconsole.js @@ +3114,5 @@ > Cu.reportError("Evaluation error " + response.error + ": " + > response.message); > return; > } > + let errorMessage = typeof(response.exception) === "string" ? new Error(response.exceptionMessage).toString() : response.exceptionMessage; This generally makes sense (although it took me a while to work through all the variations of RDP traffic to confirm exactly what cases this was covering). I think it needs some explanation in a comment though. Something like: let errorMessage = response.exceptionMessage; // Stringify thrown strings as Errors so `throw "foo"` outputs as "Error: foo" if (typeof(response.exception) === "string") { errorMessage = new Error(response.exceptionMessage).toString(); }
Attachment #8721160 -
Flags: review?(bgrinstead) → feedback+
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8721160 [details] [diff] [review] patch v1 Review of attachment 8721160 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the quick feedback! I'll explain the handling of thrown strings and add some test cases. ::: devtools/client/webconsole/webconsole.js @@ +3114,5 @@ > Cu.reportError("Evaluation error " + response.error + ": " + > response.message); > return; > } > + let errorMessage = typeof(response.exception) === "string" ? new Error(response.exceptionMessage).toString() : response.exceptionMessage; You make a good point. I was concerned it might be a bit too "magical" and I think the way you reworked it makes things a lot easier to grok at a glance.
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8721905 -
Flags: review?(bgrinstead)
Assignee | ||
Updated•8 years ago
|
Attachment #8721160 -
Attachment is obsolete: true
Comment 7•8 years ago
|
||
Comment on attachment 8721905 [details] [diff] [review] patch v2 Review of attachment 8721905 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! This needed a rebase due to Bug 1249715, but I've done that and pushed to try https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d9ed84b718d. Let's land it if that turns up green.
Attachment #8721905 -
Flags: review?(bgrinstead) → review+
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/046e5f381e66
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment 10•8 years ago
|
||
I've reproduced this bug on Nightly 36.0a1 (2014-11-14) on Ubuntu 14.04 LTS, 32-bit! This bug fixed is verified on Latest Nightly 48.0a1! Build ID: 20160308030418 User Agent: Mozilla/5.0 (X11; Linux i686; rv:48.0) Gecko/20100101 Firefox/48.0
QA Whiteboard: [bugday-20160309]
status-firefox48:
--- → verified
Comment 11•8 years ago
|
||
I have reproduced this bug according to (2014-11-14) It's fixed on Latest Developer Edition -- Build ID (20160408004012), User Agent: Mozilla/5.0 (Windows NT 6.3; rv:47.0) Gecko/20100101 Firefox/47.0 Tested OS-- Windows8.1 32bit [bugday-20160406]
Comment 12•8 years ago
|
||
I have reproduced this bug with Firefox nightly 36.0a1(2014-11-14)on windows 7(64 bit) I have verified this bug as fixed with Firefox aurora 47.0a2(build Id:20160415004038) User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Comment 13•8 years ago
|
||
As this bug is verified on both Linux (comment 10) and Windows (Comment 11), I am marking this as verified!
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•