1584244 - Getter properties with hyphenated names included in objects exported to a userScript are undefined when their value is retrieved from the webconsole interactive Object Inspector

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: fm.zip

This issue has been splitted from Bug 1583159, the attached files contains a test extension which can consistently reproduce the issue.

Steps to reproduce:

• install the test extension temporarily
• open a new tab and navigate it to a webpage that matches for the userScript registered by the test extension (e.g. https://developer.mozilla.org)
• open the webconsole, look for the object inspector part of the console.log emitted from the userScript (which should be the last console log emitted, if the webpage doesn't emit other logs on its own) and click on the getters to resolve their values

Expected behavior:

• the getter values should all be defined and have the expected values

Actual behavior:

• the getters without an hyphen character in their name have the expected values, whereas any of getter that has an hyphen character in its name seems to have an unexpected undefined value

Based on an initial investigation, the issue seems to be somehow on the devtools' object inspector, because if the userScript code access the hyphenated property names right before logging the object, e.g. by applying the following change to the attached test extension:

--- background.js       2019-09-25 16:18:34.000000000 +0200
+++ background-changed.js       2019-09-25 15:48:51.032252274 +0200
@@ -17,7 +17,7 @@
   // --- preppare script options
   const options = {

-    js: [{code: 'console.log("testing");console.log(GM.info);' }],
+    js: [{code: 'console.log("testing");console.log("DEBUG", GM.info["run-at"], GM.info["one-two"], GM.info);' }],
     matches: ['*://*/*'],
     runAt: 'document_idle'
   };

then the object inspector shown all the property values as expected, included the hyphenated ones.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

it looks like it's also an issue in the console.

Simpler STR:

1. Evaluate the following:

x = {
  get ["hello-world"]() { return "🌍"}
}

2. Expand the object in the console
3. Click on the "invoke getter" button

Expected result:

the getter result is displayed (i.e. "🌍")

Actual result:

the getter resolves to undefined

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1584244 - Fix invoke getter action on quoted property. r=Honza.

When clicking the invoke button on a property that
needs to be quoted, e.g. hello-world, the result
was undefined.
That's because we were sending the property name to
the server with the surrounding quotes.
This patch fixes the issue and adds a couple of test
case to make sure we don't regress.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/046a8c9987d9
Fix invoke getter action on quoted property. r=Honza.

Comment 4

[Rob Wu [:robwu]]

The patch doesn't look right to me. It solves the issue for the specific test case that was reported here, but it does not address other classes of problems that are related to the implementation. Could you also fix those?

I can construct a similar test case that fails despite the patch, by running the following code in the console, expanding the object and clicking on the getter:

({get "\\\"\\'\\`"(){ return 1 }})

It might not be as readable, but the actual property name is:

\"\'\`

When I attach a debugger to the devtools, and then add a logpoint in reps.js at the invokeGetter method with node.name, then I see the following output BEFORE your patch:

"\\\"\\'\\`"

AFTER the patch the output for a logpoint for node.name would obviously be the same, but for getterName the output would be:

\"\\'\\`

clearly, the output is incorrect (every backslash is repeated twice).

At the very least, instead of manually parsing the escape sequences in the string, I would use JSON.parse(item.name) if the property is quoted.

But I think that the implementation would still be flawed - there should not be a need to serialize the property key, as not everything is serializable. For example, with the following test case, item.name is Symbol(symbol), and that can clearly not be used to reconstruct the property key:

({get [Symbol.for("symbol")](){ return 1 }})

If more efficiently, perhaps the property name can "manually" be deserialized, and a (slow) path can be used if the property name cannot be deserialized.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

I'll file a follow up for a proper fix.
Thanks Rob!

Comment 6

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/046a8c9987d9

Comment 7

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/046a8c9987d9

Comment 8

[Rob Wu [:robwu]]

Could you link this bug to the bug that you've filed in response to comment 4 ?

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Sure, here it is: Bug 1585529

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

I also filed Bug 1585549 to fix Symbol getters.