Incorrect representation of sparse arrays in WebConsole
Categories
(DevTools :: Console, defect, P3)
Tracking
(firefox74 fixed)
Tracking | Status | |
---|---|---|
firefox74 | --- | fixed |
People
(Reporter: 709922234, Assigned: sidvishnoi)
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:73.0) Gecko/20100101 Firefox/73.0
Steps to reproduce:
WebConsole typeing new Array(4)
Actual results:
show Array(4) [ undefined, undefined, undefined, undefined ]
Expected results:
Representing a sparse array
Comment 1•4 years ago
|
||
The priority flag is not set for this bug.
:nchevobbe, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Hi!
I dived into this issue and found in reps/grip-array.js
, that itemGrip
was never null
(in arrayIterator
), so emptySlot
count was never incremented.
Replacing items.push(hooks.createValueGrip(undefined))
with items.push(null)
(https://searchfox.org/mozilla-central/source/devtools/server/actors/object/previewers.js#193) seemed to fix the issue, but it appears to have caused some issue with console.table
(https://bugzilla.mozilla.org/show_bug.cgi?id=1565781).
I found const desc = Object.getOwnPropertyDescriptor(Cu.waiveXrays(raw), i);
is undefined when we've an emptySlot, so following changeset appears to fix the problem:
// devtools/server/actors/object/previewers.js
if (desc && !desc.get && !desc.set) {
let value = Cu.unwaiveXrays(desc.value);
value = ObjectUtils.makeDebuggeeValueIfNeeded(obj, value);
items.push(hooks.createValueGrip(value));
+ } else if (!desc) {
+ items.push(null);
} else {
items.push(hooks.createValueGrip(undefined));
}
Should I send a patch? Would it be a good first issue?
Comment 3•4 years ago
|
||
Hello Sid, thanks for investigating.
You can definitely send a patch, that would be much appreciated.
I'm going to assign the bug to you
Assignee | ||
Comment 4•4 years ago
|
||
Hi... I can't find where to add test for this.
The closest I've reached is client/webconsole/test/browser/browser_webconsole_console_table.js
, but there is nothing for regular console.log
. Then I thought client/debugger/packages/devtools-reps/src/reps/tests/array.js
should have worked, but it shows outdated results. Do I need to run some build step (I did run mach build
)?
Comment 5•4 years ago
|
||
Then I thought client/debugger/packages/devtools-reps/src/reps/tests/array.js should have worked, but it shows outdated results
The test would be in devtools/client/debugger/packages/devtools-reps/src/reps/tests/grip-array.js (array.js tests non-grip arrays.)
The issue with the test is that it consumes stubs (devtools/client/debugger/packages/devtools-reps/src/reps/stubs/grip-array.js ), which are hand-written :/
What could be done is to add a unit test (similar to the other ones we have, like devtools/server/tests/unit/test_objectgrips-17.js ), to check we return null
for empty slots, but that we do respect undefined
item in arrays.
Let me know how this goes :)
Assignee | ||
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
I'm not sure why xpcshell test failed locally. I'm getting the following output:
0:40.76 INFO (xpcshell/head.js) | test finished (2)
0:40.76 PASS - The grip has an Array class - "Array" === "Array"
0:40.76 PASS - The empty slot has null as grip preview - null === null
0:40.76 PASS - The undefined value has grip value of type undefined - {"type":"undefined"} deepEqual {"type":"undefined"}
... then after a moment...
0:40.88 INFO (xpcshell/head.js) | test finished (2)
0:40.88 PASS - The grip has an Array class - "Array" === "Array"
0:40.88 FAIL - The empty slot has null as grip preview - {"type":"undefined"} === null
I've started a try run to check if it's my local setup failing or something else: https://treeherder.mozilla.org/#/jobs?repo=try&revision=85adc7212495ef5ccdec337fb27ade00cbcb841a
Assignee | ||
Comment 8•4 years ago
|
||
Update: Oh. It's failing against worker.
>>> Run thread front test against a worker DebuggerServer
was lost in logs.
But I'm am not able to understand how to fix that. test_objectgrips-17.js
suggests I need to run tests against systemPrincipal
and nullPrincipal
and then with/without XRays... but I don't understand how to use and which ones to use (and why).
Comment 9•4 years ago
|
||
I tried to answer in Phabricator, let me know how this turns out :)
Comment 10•4 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7dacd86c6a96 Use null for empty slots in Array grip preview. r=nchevobbe
Comment 11•4 years ago
|
||
bugherder |
Description
•