1944326 - JSTracer preview showing the wrong value

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Holger Benl]

Attachment: JSTracer shows wrong event.png

I tried to analyze mouse event handling on https://www.eclecticgames.co.uk/ (for 1943411): I recorded a trace while the mouse is moving over the menu and looked at the calls to the jQuery event handler for the mouse events in the trace. When I select one of the calls and look at the event in the Tracer preview, it seems to be showing the correct event (at least the event's type is correct). But when I select another call and look at the event in the Tracer preview I always see the same event that I saw for the first call that I selected.
The attached screenshot shows a mouseout event in the preview even though I have selected the call for a mouseover event.

Comment 1

[Alexandre Poirot [:ochameau]]

Reproducible on simple test:
data:text/html,<script>window.onmousemove=e=>console.log(e.clientX)</script>

Comment 2

[Alexandre Poirot [:ochameau]]

We do display at the top of the preview a warning, that preview popup show the current state of object.
There is room for improvement here, we could try showing only what is stored in reps object, but this require some UI work.

One the following test page, when inspecting foo, you can see that it works fine with a number as argument:
data:text/html,<script>window.onmousemove=%0Ae=>foo(e.clientX);function foo(v)%0A{}</script>
You can also see that inline previews, as they only display reps object are also correct.

May be the warning message should be in red or more visible?

Comment 3

[Holger Benl]

(In reply to Alexandre Poirot [:ochameau] from comment #2)

We do display at the top of the preview a warning, that preview popup show the current state of object.
[...]
May be the warning message should be in red or more visible?

I was aware of that (and got reminded by the warning message - I think it's prominent enough).
The inline preview indeed shows the correct value but the preview popup is always showing the same value for me.

Comment 4

[Holger Benl]

Attachment: Different values in inline preview and popup.png

In this screenshot you can see how the inline preview and the preview popup show different values for the same argument (note the clientX/clientY values of the event).

Comment 5

[Alexandre Poirot [:ochameau]]

Yes sorry, you are right, for some reason I though that e was always refering to the same object and so would justify this behavior.
But it is not, we get a brand new MouseEvent object, which should be trigerring a new ObjectActor and ultimately display different object to be rendered as preview tooltip.
That's weird, I'll have another look.

Comment 6

[Alexandre Poirot [:ochameau]]

So there is something surprising with the ObjectInspector.
We do pass the right root over there:
https://searchfox.org/mozilla-central/rev/dd8b64a6198ff599a5eb2ca096845ebd6997457f/devtools/client/debugger/src/components/Editor/Preview/Popup.js#138
But for some reason, React still renders the previously shown preview object.

So the behavior is that we should show the right attributes values the first time, and then, it won't update.

It has to do with object actors as moving from object to number will update correctly:
data:text/html,<script>window.onclick=()=>{foo({a:1}); foo(42) }; function foo(o) {};</script>
whereas any object to object won't update:
data:text/html,<script>window.onclick=()=>{foo({a:1}); foo({b:1}) }; function foo(o) {};</script>

Comment 7

[Alexandre Poirot [:ochameau]]

Regular previews are not subject to such issue whereas this is almost the same codepath:
data:text/html,<script>let a = {a:1}; let b = {b:2}; debugger;</script>

Comment 8

[Alexandre Poirot [:ochameau]]

Oh got it.

That's because the tracer preview will always set the same path attribute over there:
https://searchfox.org/mozilla-central/rev/dd8b64a6198ff599a5eb2ca096845ebd6997457f/devtools/client/debugger/src/actions/preview.js#114
It will set the hovered variable name.
And then ObjectInspector uses that as a React key attribute:
https://searchfox.org/mozilla-central/rev/dd8b64a6198ff599a5eb2ca096845ebd6997457f/devtools/client/shared/components/object-inspector/components/ObjectInspector.js#208-210
which prevents proper re-rendering.

It is surprising that the popups update correctly on stepping on such example:
data:text/html,<script>window.onclick=()=>{let a = {a:1}; debugger; a = {b:1}; debugger;}</script>
as paused preview root will also use a constant path attribute.

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1944326 - [devtools] Fix previews not updating on same-name variables.

For some reason, the ObjectInspector doesn't update when using same path
and ultimately same React element key attribute.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5332a49862e3
[devtools] Fix previews not updating on same-name variables. r=devtools-reviewers,nchevobbe

Comment 11

[amarc]

https://hg.mozilla.org/mozilla-central/rev/5332a49862e3