1408940 - Enable browser_webconsole_history_nav.js in the new console frontend

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408940 - enable browser_webconsole_history_nav.js;

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

Comment 2

[Julian Descottes [:jdescottes]]

Comment on attachment 8933695 [details]
Bug 1408940 - enable browser_webconsole_history_nav.js;

https://reviewboard.mozilla.org/r/204630/#review210220

::: commit-message-a21f4:1
(Diff revision 1)
> +Bug 1408940 - enable browser_webconsole_history_nav.js;r=nchevobbe

This one might be worth renaming to jsterm_*?

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8933695 [details]
Bug 1408940 - enable browser_webconsole_history_nav.js;

https://reviewboard.mozilla.org/r/204630/#review210220

> This one might be worth renaming to jsterm_*?

yes, i think it should :)

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8933695 [details]
Bug 1408940 - enable browser_webconsole_history_nav.js;

https://reviewboard.mozilla.org/r/204630/#review210498

Looks good, thanks Julian.
One minor comment (on a comment :)), but this is good to land/

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_history_nav.js:43
(Diff revision 1)
> +  // We don't have an explicit event to wait for here, so we just wait for a bit before
> +  // checking the popup status.
> +  await new Promise(executeSoon);

aren't we waiting for the next tick here ? It would be more accurate than saying "a bit", which may sound error prone.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8933695 [details]
Bug 1408940 - enable browser_webconsole_history_nav.js;

https://reviewboard.mozilla.org/r/204630/#review210500

::: devtools/client/webconsole/new-console-output/test/mochitest/browser.ini:297
(Diff revision 1)
>  [browser_webconsole_highlighter_console_helper.js]
>  skip-if = true #	Bug 1404853
>  # old console skip-if = true # Requires direct access to content nodes
>  [browser_webconsole_history_arrow_keys.js]
>  skip-if = true # Bug 1408939
>  [browser_webconsole_history_nav.js]

oh, and rename the test

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8933695 [details]
Bug 1408940 - enable browser_webconsole_history_nav.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/204630/diff/1-2/

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8933695 [details]
Bug 1408940 - enable browser_webconsole_history_nav.js;

https://reviewboard.mozilla.org/r/204630/#review210498

Thanks for the review Nicolas. Updated the patch, landing.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/581fec9ba4f2
enable browser_webconsole_history_nav.js;r=nchevobbe

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/581fec9ba4f2