Closed Bug 1408920 Opened 2 years ago Closed 2 years ago

Enable browser_webconsole_autocomplete_in_debugger_stackframe.js in the new console frontend

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox58 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Blocks: 1400847
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
hg error in cmd: hg pull gecko -r 7e17e3edb7f3e0e9a0aee3f7f7af0f727ec8dfb2: pulling from https://reviewboard-hg.mozilla.org/gecko
abort: HTTP Error 504: Gateway Timeout
Comment on attachment 8924904 [details]
Bug 1408920 - Rename and enable browser_webconsole_autocomplete_in_debugger_stackframe.js; .

https://reviewboard.mozilla.org/r/196172/#review201370

Some nits but this looks good and the test seems to work well!
Thanks Nicolas.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_autocomplete_cached_results.js:21
(Diff revision 1)
>      autocompletePopup: popup,
>      completeNode,
>      inputNode: input,
>    } = jsterm;
>  
> -  const jstermComplete = (value, delta) => complete(jsterm, input, value, delta);
> +  const jstermComplete = (value, off) => jstermSetValueAndComplete(jsterm, value, off);

nit: off -> offset

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_autocomplete_in_debugger_stackframe.js:63
(Diff revision 1)
> -  yield pauseDebugger(dbg);
> +  const stackFrames = await pauseDebugger(panel);
>  
>    info("Opening Console again");
> -  yield openConsole();
> +  await toolbox.selectTool("webconsole");
>  
>    // From this point on the

remove this line

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_autocomplete_in_debugger_stackframe.js:64
(Diff revision 1)
>  
>    info("Opening Console again");
> -  yield openConsole();
> +  await toolbox.selectTool("webconsole");
>  
>    // From this point on the
> -  // Test if 'f' gives 'foo3' and 'foo1' but not 'foo2'
> +  // Test if 'foo' gives 'foo3' and 'foo1' but not 'foo2'

Can you add a comment explaining that we are in the frame corresponding to secondCall() here.

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_autocomplete_in_debugger_stackframe.js:71
(Diff revision 1)
> -    return item.label != "foo2Obj";
> -  }), "autocomplete results do not contain foo2Obj");
>  
> -  yield openDebugger();
> +  await openDebugger();
>  
> -  gStackframes.selectFrame(1);
> +  stackFrames.selectFrame(1);

Add a comment explaining that we are selecting the frame for firstCall()

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_autocomplete_in_debugger_stackframe.js:76
(Diff revision 1)
> -  gStackframes.selectFrame(1);
> +  stackFrames.selectFrame(1);
>  
>    info("openConsole");
> -  yield openConsole();
> +  await toolbox.selectTool("webconsole");
>  
>    // Test if 'f' gives 'foo2' and 'foo1' but not 'foo3'

nit: Test if 'f' -> Test if 'foo'

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_autocomplete_in_debugger_stackframe.js:112
(Diff revision 1)
>    return new Promise(resolve => {
> -    thread.addOneTimeListener("framesadded", resolve);
> +    thread.addOneTimeListener("framesadded", () =>
> +      resolve(debuggerController.StackFrames));
>  
>      info("firstCall()");
> -    ContentTask.spawn(gBrowser.selectedBrowser, {}, function* () {
> +    ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {

nit: eslint "function()" -> "function ()"

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:324
(Diff revision 1)
> + *         the open fails. The resolution callback is given one argument, an
> + *         object that holds the following properties:
> + *         - target: the Target object for the Tab.
> + *         - toolbox: the Toolbox instance.
> + *         - panel: the jsdebugger panel instance.
> + *         - panelWin: the window object of the panel iframe.

panelWin is not returned here

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:333
(Diff revision 1)
> +    options.tab = gBrowser.selectedTab;
> +  }
> +
> +  let target = TargetFactory.forTab(options.tab);
> +  let toolbox = gDevTools.getToolbox(target);
> +  let dbgPanelAlreadyOpen = toolbox && toolbox.getPanel("jsdebugger");

You should select the debugger as well here. getPanel("jsdebugger") will not select the panel if it is already loaded but in the background.

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:338
(Diff revision 1)
> +  let dbgPanelAlreadyOpen = toolbox && toolbox.getPanel("jsdebugger");
> +  if (dbgPanelAlreadyOpen) {
> +    return {
> +      target,
> +      toolbox,
> +      panel: toolbox.getCurrentPanel()

re: previous comment, getCurrentPanel is not necessarily the debugger here.

::: devtools/client/webconsole/new-console-output/test/mochitest/head.js:344
(Diff revision 1)
> +    };
> +  }
> +
> +  toolbox = await gDevTools.showToolbox(target, "jsdebugger");
> +  let panel = toolbox.getCurrentPanel();
> +  panel._view.Variables.lazyEmpty = false;

If you know, can you add a comment here explaining why this is needed, and what it does?
Attachment #8924904 - Flags: review?(jdescottes) → review+
Thanks for the review Julian, I think I addressed everything.
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8823f3335102
Rename and enable browser_webconsole_autocomplete_in_debugger_stackframe.js; r=jdescottes.
https://hg.mozilla.org/mozilla-central/rev/8823f3335102
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.