Open Bug 1442604 Opened 6 years ago Updated 2 years ago

Add tests for the select and autocompletion popups used in an extension devtools panels

Categories

(WebExtensions :: Developer Tools, enhancement, P3)

60 Branch
enhancement

Tracking

(firefox60 affected)

Tracking Status
firefox60 --- affected

People

(Reporter: rpl, Unassigned)

References

Details

Attachments

(2 files)

This issue is a follow up of Bug 1417043, which is going to reuse the webext-panels.xul document to share with the extension sidebar pages the fixes related to the select and autocompletion popups when the extensions run in oop mode.

These fixes should be provided of additional automated tests to prevent it from regressing.
Priority: -- → P2
Priority: P2 → P3
Attachment #8969728 - Flags: review?(aswan)
Attachment #8969729 - Flags: review?(aswan)
Blocks: 1417346
Comment on attachment 8969728 [details]
Bug 1442604 - Split WebExtension devtools theme API tests in a separate test file.

https://reviewboard.mozilla.org/r/238526/#review246600
Attachment #8969728 - Flags: review?(aswan) → review+
Comment on attachment 8969729 [details]
Bug 1442604 - Add tests for the select and autocompletion popups used in an extension devtools panels.

https://reviewboard.mozilla.org/r/238528/#review246604

Why all the separate tasks?  I usually think of tasks as independent tests, but the tasks here are all closely related (ie, they share a bunch of state in global vars)
I guess that's just a stylistic preference but putting this into a single task would be more consistent with our other existing tests.

::: browser/components/extensions/test/browser/browser_ext_devtools_panel_dropdowns.js:22
(Diff revision 2)
> +let panelBrowser;
> +let tab;
> +
> +async function waitForExpectedValue({selector, value}) {
> +  const el = this.content.document.querySelector(selector);
> +  await ContentTaskUtils.waitForCondition(

nit: this function doesn't need to be async and can just return the result of `waitForCondition` here

::: browser/components/extensions/test/browser/browser_ext_devtools_panel_dropdowns.js:30
(Diff revision 2)
> +}
> +
> +async function ensureElementFocused(panelBrowser, elementSelector) {
> +  panelBrowser.focus();
> +  // Ensure that the devtools panel is focused.
> +  await ContentTask.spawn(panelBrowser, {

same comment as above

::: browser/components/extensions/test/browser/browser_ext_devtools_panel_dropdowns.js:50
(Diff revision 2)
> +      browser.test.notifyPass("devtools_panel_created");
> +      throw err;

Why not just notifyFail()?
Actually, I thought we conventionally used notifyPass and notifyFail at the end of a test, how about just a regular sendMessage() here?

::: browser/components/extensions/test/browser/browser_ext_devtools_panel_dropdowns.js:123
(Diff revision 2)
> +  // because we can use the same strategy to wait the different
> +  // dropdown's popups to be opened (in both extension oop and non-oop modes).
> +  SpecialPowers.pushPrefEnv({
> +    "set": [["dom.select_popup_in_parent.enabled", true]],
> +  });
> +  registerCleanupFunction(() => SpecialPowers.popPrefEnv());

I don't think you need this -- it happens implicitly
Comment on attachment 8969729 [details]
Bug 1442604 - Add tests for the select and autocompletion popups used in an extension devtools panels.

https://reviewboard.mozilla.org/r/238528/#review250482

Clearing r? until the next revision is ready
Attachment #8969729 - Flags: review?(aswan)
Product: Toolkit → WebExtensions
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: