1436110 - Throwing an empty string displays "undefined" in the console

Status: VERIFIED FIXED

(DevTools :: Console, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce:
1. Open the console
2. Evaluate `throw "";`


Expected results:

There's an empty error message

Actual results:

There an "undefined" message, without the error style.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1436110 - Fix rendering of thrown string in evaluation results; .

If the user tried to evaluate `throw ""`, an "undefined" message was displayed
in the console output, which is wrong. Some changes needed to be made to the
messages util to better handle those cases.

Review commit: https://reviewboard.mozilla.org/r/218342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218342/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1436110 - Fix stub generation and update existing stubs; .

Review commit: https://reviewboard.mozilla.org/r/218344/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218344/

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1436110 - Add pageError and evaluationResult tests for thrown strings; .

Review commit: https://reviewboard.mozilla.org/r/218346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218346/

Comment 4

[Brian Grinstead [:bgrins]]

FYI I'm going to be away for the remainder of the week. Feel free to redirect this if you'd like - otherwise I will get to it early next week.

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8948917 [details]
Bug 1436110 - Fix stub generation and update existing stubs; .

https://reviewboard.mozilla.org/r/218344/#review225436

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8948918 [details]
Bug 1436110 - Add pageError and evaluationResult tests for thrown strings; .

https://reviewboard.mozilla.org/r/218346/#review225438

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8948916 [details]
Bug 1436110 - Fix rendering of thrown string in evaluation results; .

https://reviewboard.mozilla.org/r/218342/#review225442

The way we re-map exceptionMessage into messageText for evaluation results is kind of confusing, but at least now it's more explicit.

::: devtools/client/webconsole/new-console-output/components/message-types/EvaluationResult.js:47
(Diff revision 1)
>      parameters,
>      notes,
>    } = message;
>  
>    let messageBody;
> -  if (message.messageText) {
> +  if (typeof message.messageText !== "undefined" && message.messageText !== null) {

Is this change fixing a bug? I didn't think there'd be a case where messageText / exceptionMessage would `=== false`, so this seems unnecessary.

::: devtools/client/webconsole/new-console-output/utils/messages.js:271
(Diff revision 1)
> +  } else if (typeof exception === "string") {
> +    // Wrap thrown strings in Error objects, so `throw "foo"` outputs "Error: foo"
> +    exceptionMessage = new Error(exceptionMessage).toString();
>    }
>  
> -  const level = messageText ? MESSAGE_LEVEL.ERROR : MESSAGE_LEVEL.LOG;
> +  const level = typeof exceptionMessage !== "undefined" && exceptionMessage !== null

Ditto

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

>Is this change fixing a bug? I didn't think there'd be a case where messageText / exceptionMessage would `=== false`, so this seems unnecessary.

Yes, since `messageText` can be "", we wouldn't go into the if.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d3f2a7dfdc9
Fix rendering of thrown string in evaluation results; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/719f138bc9d8
Fix stub generation and update existing stubs; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/9ce4a74db793
Add pageError and evaluationResult tests for thrown strings; r=bgrins.

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/5d3f2a7dfdc9
https://hg.mozilla.org/mozilla-central/rev/719f138bc9d8
https://hg.mozilla.org/mozilla-central/rev/9ce4a74db793

Comment 11

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 60.0a1 (2018-02-06) on Windows 10, 64 Bit!

The bug's fix is now verified with Latest Nightly!

Build ID   20180217100053
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0

Comment 12

[Oana Botisan, Desktop QA]

I tried to reproduce the bug using an older version of Nightly (2018-02-06) on Windows 10, Ubuntu 16.04 and macOS 10.13, but after I write ' throw ""; ' in the Browser Console the result I get is just the word "Error". And I get the same behaviour on latest Nightly 60.0a1 and beta 59.0b10 using the same platforms. 

Did I miss some of the steps or did I do something wrong?

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Hello Oana,

This is only on the regular console, not the browser console (the browser console still uses an old version of the console)

Comment 14

[Oana Botisan, Desktop QA]

I am sorry. I wrote the text in the wrong console. 

I retested everything using the same steps only this time I wrote 'throw ""; ' in the Web Console. I managed to reproduce the issue on the Nightly from 2018-02-06. 
I verified everything on the same platforms as comment 12 using the latest Nightly 60.0a1 and the bug is fixed. 

According to this result and the one from comment 11, I will mark this bug as verified.

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

no worries :)