Closed Bug 1408940 Opened 2 years ago Closed 2 years ago

Enable browser_webconsole_history_nav.js in the new console frontend

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(firefox59 fixed)

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: nchevobbe, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

(Whiteboard: [newconsole-mvp])

Attachments

(1 file)

No description provided.
Blocks: 1400847
Priority: P3 → P2
Whiteboard: [newconsole-mvp]
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
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 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 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.
Attachment #8933695 - Flags: review?(nchevobbe) → review+
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 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.
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/581fec9ba4f2
enable browser_webconsole_history_nav.js;r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/581fec9ba4f2
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.