1435084 - Create a pref to load the new console frontend in the Browser Console

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Brian Grinstead [:bgrins]]

This'll allow for incremental progress towards Bug 1362023 without requiring all of the test migration / feature work to be done.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: Bug 1435084 - Create a pref to enable the new console UI in the browser console;

Review commit: https://reviewboard.mozilla.org/r/217372/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217372/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8947650 [details]
Bug 1435084 - Create a pref to enable the new console UI in the browser console;

https://reviewboard.mozilla.org/r/217372/#review223842

Looks good to me. I have a comment to simplify the code a bit, but this is not really in the scope of this bug.

::: devtools/client/webconsole/hudservice.js:232
(Diff revision 1)
> -    connect().then(getTarget).then(openWindow).then((aWindow) => {
> -      return this.openBrowserConsole(target, aWindow, aWindow)
> +    connect().then(getTarget).then(openWindow).then(({iframeWindow, chromeWindow}) => {
> +      return this.openBrowserConsole(target, iframeWindow, chromeWindow)
>          .then((aBrowserConsole) => {
>            this._browserConsoleDefer.resolve(aBrowserConsole);
>            this._browserConsoleDefer = null;
>          });
>      }, console.error.bind(console));

not really in the scope of this bug, but could this somehow be turned into async/await code ? It would be much more readable.

I also wonder if we could get read of `this._browserConsoleDefer` at the same time ? Or at least, not use defer maybe ?
The only use case seems to be here https://searchfox.org/mozilla-central/rev/f80722d4f3bfb722c5ec53880c4a7efb71285676/devtools/client/webconsole/hudservice.js#181-183 to prevent multiple browser console opening.

::: devtools/client/webconsole/new-console-output/new-console-output-wrapper.js:142
(Diff revision 1)
>            { actor, clipboardText, variableText, message,
>              serviceContainer, openSidebar, rootActorId });
>  
>          // Emit the "menu-open" event for testing.
>          menu.once("open", () => this.emit("menu-open"));
> -        menu.popup(screenX, screenY, this.toolbox);
> +        menu.popup(screenX, screenY, { doc: this.owner.chromeWindow.document });

were there a bug here ?

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> ::: devtools/client/webconsole/hudservice.js:232
> (Diff revision 1)
> > -    connect().then(getTarget).then(openWindow).then((aWindow) => {
> > -      return this.openBrowserConsole(target, aWindow, aWindow)
> > +    connect().then(getTarget).then(openWindow).then(({iframeWindow, chromeWindow}) => {
> > +      return this.openBrowserConsole(target, iframeWindow, chromeWindow)
> >          .then((aBrowserConsole) => {
> >            this._browserConsoleDefer.resolve(aBrowserConsole);
> >            this._browserConsoleDefer = null;
> >          });
> >      }, console.error.bind(console));
> 
> not really in the scope of this bug, but could this somehow be turned into
> async/await code ? It would be much more readable.
> 
> I also wonder if we could get read of `this._browserConsoleDefer` at the
> same time ? Or at least, not use defer maybe ?
> The only use case seems to be here
> https://searchfox.org/mozilla-central/rev/
> f80722d4f3bfb722c5ec53880c4a7efb71285676/devtools/client/webconsole/
> hudservice.js#181-183 to prevent multiple browser console opening.
> 

Good idea, I'll file a followup bug to clean this up.

> :::
> devtools/client/webconsole/new-console-output/new-console-output-wrapper.js:
> 142
> (Diff revision 1)
> >            { actor, clipboardText, variableText, message,
> >              serviceContainer, openSidebar, rootActorId });
> >  
> >          // Emit the "menu-open" event for testing.
> >          menu.once("open", () => this.emit("menu-open"));
> > -        menu.popup(screenX, screenY, this.toolbox);
> > +        menu.popup(screenX, screenY, { doc: this.owner.chromeWindow.document });
> 
> were there a bug here ?

Yes, in the Browser Console there is no toolbox so the context menu wouldn't open. The Menu API just expects an object with 'doc' as a key, so I'm changing it to reference the doc of the chromeWindow that gets passed in through hudservice.  This is either the new browserconsole.xul window or the toolbox.xul window depending on if it's the BC or WC.

Comment 4

[Brian Grinstead [:bgrins]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8e96e7cd856f99c803add734071c0b0c4ab2829

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d23fda609203
Create a pref to enable the new console UI in the browser console;r=nchevobbe

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/d23fda609203