883345 - Undefined values aren't properly displayed in the debugger's variables view

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Victor Porof [:vporof][:vp]]

STR:

1. Open debugger on http://htmlpad.org/debugger/
2. Click me
3. Expand the |b| variable
or
  Scroll down to inspect the undefined member of the window scope
or
  Add a "undefined" (without quotes" watch expreesion

No value is displayed for undefined variables :(

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: v1

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 762884 [details] [diff] [review]
v1

Review of attachment 762884 [details] [diff] [review]:
-----------------------------------------------------------------

This is actually a server bug. The protocol specifies how undefined values should be transferred, but I must have botched it:

https://wiki.mozilla.org/Remote_Debugging_Protocol#Grips

In this case the server's response for a prototypeAndProperties request on object "b" is:

{
  "from": "conn0.obj50",
  "prototype": {
    "type": "object",
    "class": "Object",
    "actor": "conn0.obj62",
    "extensible": true,
    "frozen": false,
    "sealed": false
  },
  "ownProperties": {
    "a": {
      "configurable": true,
      "enumerable": true,
      "writable": true,
      "value": 1
    },
    "xxx": {
      "configurable": true,
      "enumerable": true
    }
  },
  "safeGetterValues": {}
}

I bet all of our unit tests for undefined values deal with arguments, not variable properties, otherwise we should have caught this long ago. Are you going to look into this or do you want me to?

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #2)

> I bet all of our unit tests for undefined values deal with arguments, not
> variable properties, otherwise we should have caught this long ago.

Yup, that's why I added some more tests in this patch.

> Are you going to look into this or do you want me to?

Don't worry about it, I'll take a look.

Comment 4

[Victor Porof [:vporof][:vp]]

(In reply to Victor Porof [:vp] from comment #3)
> (In reply to Panos Astithas [:past] from comment #2)
> 
> > I bet all of our unit tests for undefined values deal with arguments, not
> > variable properties, otherwise we should have caught this long ago.

Variables themselves (direct members of a scope) seem to be ok as well, so this happens only for properties.

Comment 5

[Victor Porof [:vporof][:vp]]

Attachment: v1

Well.. this was pretty obvious.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 763546 [details] [diff] [review]
v1

Review of attachment 763546 [details] [diff] [review]:
-----------------------------------------------------------------

I'm... sorry!

::: toolkit/devtools/server/tests/unit/test_framebindings-01.js
@@ +56,5 @@
> +      do_check_eq(aResponse.ownProperties.b.configurable, true);
> +      do_check_eq(aResponse.ownProperties.b.enumerable, true);
> +      do_check_eq(aResponse.ownProperties.b.writable, true);
> +      do_check_eq(aResponse.ownProperties.b.value.type, "undefined");
> +      do_check_true(!("class" in aResponse.ownProperties.b.value));

do_check_false would be better here.

Comment 7

[Victor Porof [:vporof][:vp]]

Attachment: v1.1

Now using do_check_false.

Comment 8

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/6d680ab84fbd

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6d680ab84fbd