u593386 Reporter
	Description • 7 years ago

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20171003101344

Steps to reproduce:

browser.windows.getAll().then(function (windows) {
    for (let w of windows)
        console.log(w.type);
});
> normal 
> popup
> popup

browser.windows.getAll({windowTypes: ["normal"]}).then(function (windows) {
    for (let w of windows)
        console.log(w.type);
});
> normal
> popup
> popup


Actual results:

All windows are returned in both versions.


Expected results:

Only normal windows should be returned in the latter example, as per the documentation and example at: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/windows/getAll

Which also says that Firefox supports the parameter since version 45.

	Andy McKay
	Comment 1 • 7 years ago

Bob would you be able to take a look at this please?

	Shane Caraveo (:mixedpuppy)
	Comment 3 • 7 years ago
mozreview-review

Comment on attachment 8919024 [details]
Bug 1406379 - Implement windowTypes option for windows.getAll,

https://reviewboard.mozilla.org/r/189906/#review195050

::: browser/components/extensions/ext-windows.js:85
(Diff revision 1)
>          },
>  
>          getAll: function(getInfo) {
> -          let windows = Array.from(windowManager.getAll(), win => win.convert(getInfo));
> +          function typeFilter(win) {
> +            return getInfo === null
> +              || getInfo.windowTypes === null

It would be nice if we didn't evaluate all this for each window, but I expect in the real world its a non-issue.

::: browser/components/extensions/test/browser/browser_ext_windows.js:41
(Diff revision 1)
> +                            "Expect type to be normal");
> +      browser.test.assertEq("normal", wins[1].type,
> +                            "Expect type to be normal");
> +
> +      wins = await browser.windows.getAll({windowTypes: ["popup", "normal"]});
> +      browser.test.assertEq(3, wins.length, "Expect one windows");

s/one/three/

	Bob Silverberg [:bsilverberg] Assignee
	Comment 5 • 7 years ago
mozreview-review-reply

Comment on attachment 8919024 [details]
Bug 1406379 - Implement windowTypes option for windows.getAll,

https://reviewboard.mozilla.org/r/189906/#review195050

> It would be nice if we didn't evaluate all this for each window, but I expect in the real world its a non-issue.

I fixed it anyway. It makes the code a bit prettier too.

	Pulsebot
	Comment 6 • 7 years ago

Pushed by bsilverberg@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65fe9a60d39d
Implement windowTypes option for windows.getAll, r=mixedpuppy

	Stefan Hindli [:stefan_hindli]
	Comment 7 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/65fe9a60d39d

	marius.santa
	Comment 8 • 7 years ago

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.