1465717 - Missing test for opening links when devtools are in a separate window

Status: RESOLVED FIXED

(DevTools :: Application Panel, defect)

Description

[Belén [:ladybenko]]

Follow up to bug https://bugzilla.mozilla.org/show_bug.cgi?id=1462232 (missing tests)

Comment 1

[Belén [:ladybenko]]

Attachment: Bug 1465717 - Missing test for opening links when devtools are in a separate window.

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

Comment 2

[Julian Descottes [:jdescottes]]

Comment on attachment 8982146 [details]
Bug 1465717 - Missing test for opening links when devtools are in a separate window.

https://reviewboard.mozilla.org/r/248168/#review254354

Looks great! Thanks for the patch.

::: devtools/client/application/test/browser_application_panel_open-links.js:9
(Diff revision 1)
> +"use strict";
> +
> +let { Toolbox } = require("devtools/client/framework/toolbox");
> +
> +/**
> + * Check that links work when the devtools are dettached in a separate window.

nit: dettached -> detached

::: devtools/client/application/test/browser_application_panel_open-links.js:24
(Diff revision 1)
> +    toolbox
> +  } = await openNewTabAndApplicationPanel(TAB_URL);
> +
> +  let doc = panel.panelWin.document;
> +
> +  // deattach devtools in a separate window

nit: deattach -> detach

Could consider making this an info() rather than a comment. I like having info() logs before every await that might timeout.

::: devtools/client/application/test/browser_application_panel_open-links.js:28
(Diff revision 1)
> +  let onTabLoaded = BrowserTestUtils
> +    .waitForNewTab(gBrowser, "about:debugging#workers");
> +  doc.querySelector(".js-trusted-link").click();
> +  let newTab = await onTabLoaded;

Maybe add an info() before the `await` ?

Comment 3

[Belén [:ladybenko]]

Comment on attachment 8982146 [details]
Bug 1465717 - Missing test for opening links when devtools are in a separate window.

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

Comment 4

[Belén [:ladybenko]]

Comment on attachment 8982146 [details]
Bug 1465717 - Missing test for opening links when devtools are in a separate window.

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

Comment 5

[Code Review Bot [:reviewbot]]

Comment on attachment 8982146 [details]
Bug 1465717 - Missing test for opening links when devtools are in a separate window.

https://reviewboard.mozilla.org/r/248168/#review255498


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/client/application/src/components/WorkerListEmpty.js:74
(Diff revision 3)
>          },
>            li({ className: "worker-list-empty__tips__item" })
>          ),
>          Localized({
>            id: "serviceworker-empty-suggestions-aboutdebugging",
> -          a: a({ className: "link", onClick: () => this.openAboutDebugging() })
> +          a: a({ className: "link js-trusted-link", onClick: () => this.openAboutDebugging() })

Error: Line 74 exceeds the maximum line length of 90. [eslint: max-len]

Comment 6

[Belén [:ladybenko]]

Comment on attachment 8982146 [details]
Bug 1465717 - Missing test for opening links when devtools are in a separate window.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248168/diff/3-4/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c04858ec3b6
Missing test for opening links when devtools are in a separate window. r=jdescottes

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7c04858ec3b6