1608369 - console input is eager evaluated twice on each key stroke

Status: VERIFIED FIXED

(DevTools :: Console, defect, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Jan-10-2020 11-08-19.mp4

Steps to reproduce

1. Make sure the eager evaluation pref is set to true
2. Open the console
3. Type Math.random()
4. Wait for a result to be shown
5. Type an additional space so the input gets eagerly evaluated again

Expected results

The result is updated once

Actual results

The result is updated twice. It's very visible with Math.random as we're getting different results.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1608369 - Don't trigger eager evaluation if the input was the same than for previous evaluation. r=bhackett.

Before trying to reach the server, we check that the new
expression is different from the previous one.
If it's similar, we bail.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23688b6ac3de
Don't trigger eager evaluation if the input was the same than for previous evaluation. r=bhackett.

Comment 3

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/23688b6ac3de

Comment 4

[Simona Badau, Desktop QA]

Attachment: 2020-02-14_11h17_23.avi

I reproduced the initial issue on Nightly 74.0a1 (20200109094415).
I can confirm the issue is no longer reproducible when following the steps from the Description (verified on Ubuntu 18.04, Mac OS X 10.15 and Windows 10 x64 using the latest Firefox 74 beta 3 and the latest Nightly 75.0a1), but I came across 2 situations where the results are still updated several times:
-1. if space is added between the parenthesis, the number is updated with each made space
-2. after pressing enter, the number is updated.
Please see the video for more details.

Nicolas, are these results, expected?

Comment 5

[Simona Badau, Desktop QA]

Attachment: Screencast

Sorry for that, added the video again since the previous format is not supported.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Hello Simona,

For 1. , that would be hard to not re-evaluate. We are already trimming the input and checking it's not the same as the previous one. But when the whitespace characters are not at the start and/or beginning of the input, that's more tricky to do
For 2. , yes, this is "expected". The eager evaluation is done at t time. If you evaluate (i.e., press Enter) at t + X time, the expression might not be the same (and we do want that, you can think of new Date() as a good example).

Comment 7

[Simona Badau, Desktop QA]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #6)

Hello Simona,

For 1. , that would be hard to not re-evaluate. We are already trimming the input and checking it's not the same as the previous one. But when the whitespace characters are not at the start and/or beginning of the input, that's more tricky to do

Nicolas, should I log a new defect for issue 1?

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Simona Badau from comment #7)

Nicolas, should I log a new defect for issue 1?

No, I think that's fine.
The work (and perf impact) it would take to not have this would outweigh the benefits.

Comment 9

[Simona Badau, Desktop QA]

Based on Comment 5, Comment 6 and Comment 8, setting the tracking flag for firefox 74 and the tracking status to Verified.