Closed Bug 1584244 Opened 4 months ago Closed 4 months ago

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

Categories

(DevTools :: Console, defect)

defect
Not set

Tracking

(firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: rpl, Assigned: nchevobbe)

References

Details

Attachments

(2 files)

Attached file 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.

See Also: → 1583159

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

Simpler STR:

  1. Evaluate the following:
x = {
  get ["hello-world"]() { return "🌍"}
}
  1. Expand the object in the console
  2. Click on the "invoke getter" button

Expected result:

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

Actual result:

the getter resolves to undefined

Component: Developer Tools → Console
Product: WebExtensions → DevTools

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.

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/046a8c9987d9
Fix invoke getter action on quoted property. r=Honza.

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.

Flags: needinfo?(nchevobbe)

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

Flags: needinfo?(nchevobbe)
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

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

Flags: needinfo?(nchevobbe)
Blocks: 1585529

Sure, here it is: Bug 1585529

Flags: needinfo?(nchevobbe)

I also filed Bug 1585549 to fix Symbol getters.

QA Whiteboard: [qa-71b-p2]
You need to log in before you can comment on or make changes to this bug.