1401942 - Migrate browser_bug664688_sandbox_update_after_navigation.js to the new console

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

It seems that the test wait for "messages-cleared" undefinitely.
I don't think we emit this event on browser navigation now.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1401942 - Call jsterm clearOutput from new-webconsole client;

The local clearOutput defined in new-webconsole.js was only used in one
place, in the same file, on tab navigation. It would probably be best to
reuse the same code path as in other places, ie jsterm.clearOutput().

(plus the clear storage argument of the local method was never used).

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

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1401942 - Rename and enable browser_bug664688_sandbox_update_after_navigation.js;

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

Comment 4

[Julian Descottes [:jdescottes]]

If using jsterm.clearOutput() proves to be an issue, I can workaround this. Just thought it would be nice to reuse existing code here.

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

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8932988 [details]
Bug 1401942 - Rename and enable browser_bug664688_sandbox_update_after_navigation.js;

https://reviewboard.mozilla.org/r/203932/#review209776

Small nit about the test name, but this looks good, thanks Julian !

::: devtools/client/webconsole/new-console-output/test/mochitest/browser.ini:223
(Diff revision 1)
>  [browser_jsterm_no_autocompletion_on_defined_variables.js]
>  [browser_jsterm_no_input_and_tab_key_pressed.js]
>  [browser_jsterm_no_input_change_and_tab_key_pressed.js]
>  [browser_jsterm_popup_close_on_tab_switch.js]
>  [browser_jsterm_popup.js]
> +[browser_jsterm_sandbox_update_after_navigation.js]

sorry,  i'm going to be annoying, could we keep the webconsole_ prefix (and not use the jsterm_ one) ?
Even if the test do use the jsterm, we are more checking a general console behaviour.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8932987 [details]
Bug 1401942 - Call jsterm clearOutput from new-webconsole client;

https://reviewboard.mozilla.org/r/203930/#review209780

Manually tested it and it seems good. Then change makes sens in my opinion, I wonder if there were a specific reason to have a dedicated function in here.
If try is green, let's land it

Comment 7

[Julian Descottes [:jdescottes]]

Thanks for the reviews, try looks green so I'll land that after re-renaming the test :)

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8932988 [details]
Bug 1401942 - Rename and enable browser_bug664688_sandbox_update_after_navigation.js;

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67f973fb1164
Call jsterm clearOutput from new-webconsole client;r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/97f3b9b875d7
Rename and enable browser_bug664688_sandbox_update_after_navigation.js;r=nchevobbe

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/67f973fb1164
https://hg.mozilla.org/mozilla-central/rev/97f3b9b875d7