Comment on attachment 8789933 [details] [diff] [review] sparse-pseudo-arrays-v1.patch Review of attachment 8789933 [details] [diff] [review]: ----------------------------------------------------------------- Another great patch :)

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/fx-team/rev/4cba7ff88539 Allow sparse pseudo-arrays in console. r=fitzgen

Backed out for timing out in devtools test browser_webconsole_output_05.js: https://hg.mozilla.org/integration/fx-team/rev/8b57e28ff342 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=4cba7ff88539a9b47628cd86d0eb86199a7d0077 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=11517258&repo=fx-team 20:07:50 INFO - 435 INFO TEST-PASS | devtools/client/webconsole/test/browser_webconsole_output_05.js | matched rule: JS eval output: undefined - 20:07:50 INFO - 436 INFO Clicking 20:07:50 INFO - 437 INFO TEST-PASS | devtools/client/webconsole/test/browser_webconsole_output_05.js | the message body - 20:07:50 INFO - 438 INFO message body tagName 'a' className 'cm-variable' 20:07:50 INFO - 439 INFO Variables view opened 20:07:50 INFO - 440 INFO TEST-PASS | devtools/client/webconsole/test/browser_webconsole_output_05.js | variables view was shown - 20:07:50 INFO - 441 INFO checkInput(14): new Object({1: 'this\nis\nsupposed\nto\nbe\na\nvery\nlong\nstring\n,shown\non\na\nsingle\nline', 2: 'a shorter string', 3: 100}) 20:07:50 INFO - 442 INFO Logging 20:07:50 INFO - 443 INFO Waiting for messages... 20:07:50 INFO - 444 INFO Longer timeout required, waiting longer... Remaining timeouts: 1 20:07:50 INFO - 445 INFO TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_output_05.js | Test timed out - 20:07:50 INFO - --DOCSHELL 0x7fd67df96000 == 10 [pid = 1530] [id = 925] 20:07:50 INFO - --DOCSHELL 0x7fd67a911000 == 9 [pid = 1530] [id = 924] 20:07:50 INFO - 446 INFO Removing tab. 20:07:50 INFO - 447 INFO Waiting for event: 'TabClose' on [object XULElement]. 20:07:50 INFO - 448 INFO Got event: 'TabClose' on [object XULElement]. 20:07:50 INFO - 449 INFO Tab removed and finished closing 20:07:50 INFO - console.log: dumpConsoles start 20:07:50 INFO - console.log: dumpConsoles end 20:07:50 INFO - Not taking screenshot here: see the one that was previously logged 20:07:50 INFO - 450 INFO TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_output_05.js | failed to match rule: console.log() output: Object { 1: "this is supposed to be a very long …", 2: "a shorter string", 3: 100 } - 20:07:50 INFO - Stack trace: 20:07:50 INFO - chrome://mochitests/content/browser/devtools/client/webconsole/test/head.js:testCleanup:1332 20:07:50 INFO - chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:466 20:07:50 INFO - timeoutFn@chrome://mochikit/content/browser-test.js:866:9 20:07:50 INFO - setTimeout handler*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:624:12 20:07:50 INFO - timeoutFn@chrome://mochikit/content/browser-test.js:844:13 20:07:50 INFO - setTimeout handler*Tester_execTest@chrome://mochikit/content/browser-test.js:828:9 20:07:50 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:704:7 20:07:50 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:743:59

Updated that test

Pushed by kwierso@gmail.com: https://hg.mozilla.org/integration/fx-team/rev/35e1982426b2 Allow sparse pseudo-arrays in console r=fitzgen

I'm starting to think that I went too far with this. Maybe it would be better to err on the side of caution, and only allow pseudo-arrays to be sparse if they have a length property. What do you think?

(In reply to Oriol from comment #8) > I'm starting to think that I went too far with this. > Maybe it would be better to err on the side of caution, and only allow > pseudo-arrays to be sparse if they have a length property. > What do you think? That seems like a pretty good rule of thumb. Maybe either of more-than-one-key-and-all-keys-are-int32 or has-length-property-and-keys-are-int32? I don't think we need to make this logic super complicated, though. I feel diminishing returns are going to kick in real fast after having basic support.