Closed Bug 1465717 Opened 3 years ago Closed 3 years ago

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

Categories

(DevTools :: Application Panel, defect)

defect
Not set
normal

Tracking

(firefox62 fixed)

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: ladybenko, Assigned: ladybenko)

References

Details

Attachments

(1 file)

Follow up to bug https://bugzilla.mozilla.org/show_bug.cgi?id=1462232 (missing tests)
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` ?
Attachment #8982146 - Flags: review?(jdescottes) → review+
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]
Pushed by balbeza@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c04858ec3b6
Missing test for opening links when devtools are in a separate window. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/7c04858ec3b6
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.