Closed
Bug 1424721
Opened 5 years ago
Closed 5 years ago
Can't copy longString nor store it as global variable
Categories
(DevTools :: Console, defect, P2)
DevTools
Console
Tracking
(firefox59 fixed)
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: Oriol, Assigned: Oriol)
References
Details
Attachments
(1 file)
Bug 1424721 - Allow long strings and invisible-to-debugger objects to be stored as global variables.
59 bytes,
text/x-review-board-request
|
nchevobbe
:
review+
|
Details |
Enter this to the console: "a".repeat(1e4); Right-click the result and choose "Store as global variable" or "Copy object". It doesn't happen because of error occurred while processing 'evaluateJSAsync: TypeError: objActor.obj is undefined Stack: evalWithDebugger@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:1323:13 onEvaluateJS@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:917:20 onEvaluateJSAsync@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:888:20 onPacket@resource://devtools/shared/base-loader.js -> resource://devtools/server/main.js:1788:15 receiveMessage@resource://devtools/shared/base-loader.js -> resource://devtools/shared/transport/transport.js:761:7 Line: 1323, column: 13
Assignee | ||
Comment 1•5 years ago
|
||
Invisible-to-debugger objects cannot be copied either. Components.utils.Sandbox(null, {invisibleToDebugger: true})
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•5 years ago
|
||
I added a `rawValue` method to all actors in order to have a common API to obtain the underlying JS value.
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Sorry for the delay Oriol, this is a pretty busy week :)
Priority: -- → P2
Comment 5•5 years ago
|
||
mozreview-review |
Comment on attachment 8936519 [details] Bug 1424721 - Allow long strings and invisible-to-debugger objects to be stored as global variables. https://reviewboard.mozilla.org/r/207198/#review213484 ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:22 (Diff revision 1) > + await Promise.all([ContentTask.spawn(gBrowser.selectedBrowser, null, () => { > + let {window} = content; > + let obj = Cu.Sandbox(Cu.getObjectPrincipal(window), {invisibleToDebugger: true}); > + window.wrappedJSObject.invisibleToDebugger = obj; > + window.console.log("foo", obj); > + }), waitForMessage(hud, "foo")]); this is a bit hard to read, could we split this ? const onFooLog = waitForMessage(hud, "foo") ContentTask.spawn(gBrowser.selectedBrowser, null, () => { let {window} = content; let obj = Cu.Sandbox(Cu.getObjectPrincipal(window), {invisibleToDebugger: true}); window.wrappedJSObject.invisibleToDebugger = obj; window.console.log("foo", obj); }); await onFooLog; Also, could we log something else than "foo" ("invisible" ?), i'm afraid of it clashing with the logs from the script in the test uri. ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:29 (Diff revision 1) > + let messages = await waitFor(() => findMessages(hud, "foo")); > + is(messages.length, 5, "Five messages should have appeared"); i'd not wait for the invisible one in here. Since we already wait for invisible to be shown, we could get it directly, withoug using findMessages ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:67 (Diff revision 1) > + await storeAsVariable(hud, msgLongStr, "string"); > + let equality = hud.jsterm.getInputValue() + " === longString"; > + executedResult = await hud.jsterm.requestEvaluation(equality); > + is(executedResult.result, true, "Correct variable assigned into console."); can we use the same pattern than for other tests ? Or change how we check equality in other tests to we have a consistent way of testing this ? ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:68 (Diff revision 1) > ok(executedResult.textContent.includes("{ baz: 1 }"), > "Correct variable assigned into console " + executedResult.textContent); > + > + info("Check store as global variable is enabled for long strings"); > + await storeAsVariable(hud, msgLongStr, "string"); > + let equality = hud.jsterm.getInputValue() + " === longString"; could we usde window.longString so it's more obvious where it comes from ? ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:72 (Diff revision 1) > + info("Check store as global variable is enabled for invisible-to-debugger objects"); > + await storeAsVariable(hud, msgInvisible, "object"); > + equality = hud.jsterm.getInputValue() + " === invisibleToDebugger"; > + executedResult = await hud.jsterm.requestEvaluation(equality); > + is(executedResult.result, true, "Correct variable assigned into console."); same thing here ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:79 (Diff revision 1) > + equality = hud.jsterm.getInputValue() + " === invisibleToDebugger"; > + executedResult = await hud.jsterm.requestEvaluation(equality); > + is(executedResult.result, true, "Correct variable assigned into console."); > }); > > -async function storeAsVariable(hud, element) { > +let tempIdx = 0; Could we pass an expectedIndex to the storeAsVariableFunction , and handle the current tempIdx when we call it, instead of having it here ? I think it would be cleaner
I'll go through the actor changes when i have time to watch it in a calm environment :D
Assignee | ||
Comment 7•5 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #5) > Also, could we log something else than "foo" ("invisible" ?), i'm afraid of > it clashing with the logs from the script in the test uri. > can we use the same pattern than for other tests ? I would prefer not to, because the console only shows a small part of the string and I want to test that the whole string was assigned to the global variable. > Or change how we check equality in other tests to we have a consistent way of testing this ? OK, will try this. > Could we pass an expectedIndex to the storeAsVariableFunction, and handle > the current tempIdx when we call it, instead of having it here ? > I think it would be cleaner The reasoning was being able to reorder the tests without having to change the expected variable name
Comment hidden (mozreview-request) |
Comment 9•5 years ago
|
||
mozreview-review |
Comment on attachment 8936519 [details] Bug 1424721 - Allow long strings and invisible-to-debugger objects to be stored as global variables. https://reviewboard.mozilla.org/r/207198/#review213874 I have a minor comment, but this is looking good. Thanks Oriol ! ::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_context_menu_store_as_global.js:53 (Diff revision 2) > - ok(executedResult.textContent.includes("{ baz: 1 }"), > - "Correct variable assigned into console " + executedResult.textContent); > + ContentTask.spawn(gBrowser.selectedBrowser, null, () => { > + let obj = Cu.Sandbox(Cu.getObjectPrincipal(content), {invisibleToDebugger: true}); > + content.wrappedJSObject.invisibleToDebugger = obj; > + content.console.log("foo", obj); > + }); > + let msgInvisible = (await waitForMessage(hud, "foo")).node; we should create a promise for waitForMessage before firing the log, otherwise we might get a race condition. let onMessageInvisible = waitForMessage(hud, "foo") ContentTask … let msgInvisible = (await onMessageInvisible).node;
Attachment #8936519 -
Flags: review?(nchevobbe) → review+
Comment hidden (mozreview-request) |
Comment 12•5 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/440c32f8f0a9 Allow long strings and invisible-to-debugger objects to be stored as global variables. r=nchevobbe
Keywords: checkin-needed
Comment 13•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/440c32f8f0a9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Comment 14•4 years ago
|
||
I have reproduced this bug with Nightly 59.0a1 (2017-12-11) on Windows 10, 64 Bit! This bug's fix is verified with latest Nightly! Build ID : 20180120100135 User Agent : Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
QA Whiteboard: [bugday-20180117]
Updated•4 years ago
|
Product: Firefox → DevTools
Duplicate of this bug: 1410048
You need to log in
before you can comment on or make changes to this bug.
Description
•