1690611 - Add unit test that verifies that `windows.Window.title` is set without "tabs" permission depending on host permissions

Status: RESOLVED FIXED

(WebExtensions :: General, task, P5)

Description

[Rob Wu [:robwu]]

Bug 1679688 relaxed the requirement, so that the title property of the Window type of the windows extension API is available even without the "tabs" permission, provided that extensions have host permissions.

That is because the check of the title getter relies on hasTabPermission, whose implementation was improved by bug 1679688.

To make sure that the functionality behaves as expected, a unit test should be added, that confirms that the "title" property is readable when only host permissions are set, and that title is void when the extension doesn't have any permission.

The test can be added at https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js

Comment 1

[Rob Wu [:robwu]]

Are you interested in adding this one test? Don't feel obliged to, but if you're interested, you may take this bug!

Comment 2

[robbendebiene]

I don't have the time at the moment, but I'll take a look at the weekend :)

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Tabbed Browser' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 5

[robbendebiene]

Attachment: Bug 1690611 - Add tests to check if windows.Window.title is correctly set depending on the given permissions r=robwu

Comment 6

[robbendebiene]

It would be nice if you could take a look at the test, especially the 2 comments I've added in there labeled with "TODO".
I felt like the file you suggested (https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js) doesn't really fit so I've created a new one. But since this API will never be available on Android a mochitest might be the wrong place?

Comment 7

[Rob Wu [:robwu]]

(In reply to robbendebiene from comment #6)

It would be nice if you could take a look at the test, especially the 2 comments I've added in there labeled with "TODO".
I felt like the file you suggested (https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js) doesn't really fit so I've created a new one.

Why wouldn't it fit? I was thinking of virtually the same test code as currently shown in browser_ext_windows_popup_title.js, except with host permissions instead of the tabs permission. In that test, the opened window is a moz-extension:-page, for which the extension already has host permissions. So removing line 12 would be a way to resolve this bug.

To make sure that these properties are only shown when the extension has the right permission, you can extend the test with an example.com window:

• with host permissions: title/url shown
• with tabs permission: title/url shown
• without permissions: title/url hidden

But since this API will never be available on Android a mochitest might be the wrong place?

Indeed. Android doesn't support the browser.windows API.

Comment 8

[robbendebiene]

Why wouldn't it fit?

Just because this test doesn't check other methods like windows.update or onCreated. Even though they very likely work the same under the hood, in my test case the browser.windows.onCreated test failed. However this might be a wrong expectation or just a code error on my side.

Should I test the activeTab permission as well or can I ignore it?

Comment 9

[Rob Wu [:robwu]]

The title getter is used in a common implementation. Testing windows.create is sufficient.

Your test passes a tabId to the new window, which adopts the tab. I'm not sure what would happen. The title should be available if the tab has a title.

activeTab could be tested, but you don't have to.

Comment 10

[robbendebiene]

Can I somehow generate the html file so I can access it via "http://www.example.com" as well? If not how can I add and load an html file in the test. I thought adding it to the data folder would be enough because of support-files = data/** in the browser.ini. But I always get a 404 not found when I try to open it like this: "http://www.example.com/toolkit/components/extensions/test/browser/data/browser_ext_windows_popup_title.html"

Comment 11

[Rob Wu [:robwu]]

Here is an existing example: https://searchfox.org/mozilla-central/rev/8432d4fe31245ae9121231d7c0335aaa342675d2/toolkit/components/extensions/test/browser/browser_ext_downloads_referrer.js#10-11
It looks like you've omitted browser/ at the front.
(and after updating a test manifest, you may need to run ./mach build again to ensure that all changes are effective.)

A potential alternative to use a http:-URL is to have two extensions. One extension will implicitly have permissions for its own moz-extension:-page, the other will not (i.e. not have permission to access a URL from a different extension). A third extension with "tabs" permission will see it despite not having permissions. The resulting test may be even simpler than one where you open a http:-URL.

Comment 12

[robbendebiene]

It looks like you've omitted browser/ at the front.

Thanks, this was the problem.

A potential alternative to use a http:-URL is to have two extensions. One extension will implicitly have permissions for its own moz-extension:-page, the other will not (i.e. not have permission to access a URL from a different extension). A third extension with "tabs" permission will see it despite not having permissions. The resulting test may be even simpler than one where you open a http:-URL.

You are right, it would be simpler. But shouldn't I test for <all_urls> or a specific host permission as well? In this case I wouldn't be able to do that, since moz-extension: URLs are privileged.

Comment 13

[Rob Wu [:robwu]]

(In reply to robbendebiene from comment #12)

A potential alternative to use a http:-URL is to have two extensions. One extension will implicitly have permissions for its own moz-extension:-page, the other will not (i.e. not have permission to access a URL from a different extension). A third extension with "tabs" permission will see it despite not having permissions. The resulting test may be even simpler than one where you open a http:-URL.

You are right, it would be simpler. But shouldn't I test for <all_urls> or a specific host permission as well? In this case I wouldn't be able to do that, since moz-extension: URLs are privileged.

The implementation ultimately uses originsPermissions, and the implementation is equivalent for host permissions and extension permissions:

• matchesHostPermissions uses allowedOrigins - https://searchfox.org/mozilla-central/rev/8432d4fe31245ae9121231d7c0335aaa342675d2/toolkit/components/extensions/parent/ext-tabs-base.js#203
• allowedOrigins is generated from originPermissions - https://searchfox.org/mozilla-central/rev/8432d4fe31245ae9121231d7c0335aaa342675d2/toolkit/components/extensions/Extension.jsm#1258
• originPermissions always includes the moz-extension:-origin itself - https://searchfox.org/mozilla-central/rev/8432d4fe31245ae9121231d7c0335aaa342675d2/toolkit/components/extensions/Extension.jsm#1036-1038

So, you don't have to test <all_urls> or a specific host permission with my proposal.

Comment 14

[robbendebiene]

All right, I've just updated the test.
I may be wrong but I think the previous test (as well as the current) has an issue.
The name variable is defined outside of the extension context but used inside the extension (https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js#25)
I would expect that it's therefore undefined but it seems to return an empty string.

Comment 15

[Rob Wu [:robwu]]

(In reply to robbendebiene from comment #14)

I may be wrong but I think the previous test (as well as the current) has an issue.
The name variable is defined outside of the extension context but used inside the extension (https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/toolkit/components/extensions/test/browser/browser_ext_windows_popup_title.js#25)
I would expect that it's therefore undefined but it seems to return an empty string.

Good catch! window.name is an empty string by default - https://developer.mozilla.org/en-US/docs/Web/API/Window/name

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6361266a4d3
Add tests to check if windows.Window.title is correctly set depending on the given permissions r=robwu

Comment 17

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a6361266a4d3