Bug 1880078 - A previously opened tab is shown in Address Bar after restarting the browser and closing the tab. r=jteow

Some consumer is passing in integer as expected, most pass a numeric string
that is the result of a getAttribute. This causes a mismatch as that id is used
as key in a Map.
While it would be possible to fix all the consumers, the mistake seems so
common and trivial, that is worth just adapting the API to accept both.

Differential Revision: https://phabricator.services.mozilla.com/D203019
This commit is contained in:
Marco Bonardo 2024-02-29 14:52:56 +00:00
parent 751ed2987c
commit ee49ed7b17
2 changed files with 68 additions and 3 deletions

View File

@ -82,11 +82,15 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
/**
* Return unique urls that are open for given user context id.
*
* @param {integer} userContextId Containers user context id
* @param {integer|string} userContextId Containers user context id
* @param {boolean} [isInPrivateWindow] In private browsing window or not
* @returns {Array} urls
*/
static getOpenTabs(userContextId, isInPrivateWindow = false) {
// It's fairly common to retrieve the value from an HTML attribute, that
// means we're getting sometimes a string, sometimes an integer. As we're
// using this as key of a Map, we must treat it consistently.
userContextId = parseInt(userContextId);
userContextId = UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
userContextId,
isInPrivateWindow
@ -168,10 +172,22 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
* Registers a tab as open.
*
* @param {string} url Address of the tab
* @param {integer} userContextId Containers user context id
* @param {integer|string} userContextId Containers user context id
* @param {boolean} isInPrivateWindow In private browsing window or not
*/
static async registerOpenTab(url, userContextId, isInPrivateWindow) {
// It's fairly common to retrieve the value from an HTML attribute, that
// means we're getting sometimes a string, sometimes an integer. As we're
// using this as key of a Map, we must treat it consistently.
userContextId = parseInt(userContextId);
if (!Number.isInteger(userContextId)) {
lazy.logger.error("Invalid userContextId while registering openTab: ", {
url,
userContextId,
isInPrivateWindow,
});
return;
}
lazy.logger.info("Registering openTab: ", {
url,
userContextId,
@ -195,10 +211,14 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
* Unregisters a previously registered open tab.
*
* @param {string} url Address of the tab
* @param {integer} userContextId Containers user context id
* @param {integer|string} userContextId Containers user context id
* @param {boolean} isInPrivateWindow In private browsing window or not
*/
static async unregisterOpenTab(url, userContextId, isInPrivateWindow) {
// It's fairly common to retrieve the value from an HTML attribute, that
// means we're getting sometimes a string, sometimes an integer. As we're
// using this as key of a Map, we must treat it consistently.
userContextId = parseInt(userContextId);
lazy.logger.info("Unregistering openTab: ", {
url,
userContextId,

View File

@ -78,3 +78,48 @@ add_task(async function test_openTabs() {
// Sanity check that this doesn't throw.
provider.cancelQuery(context);
});
add_task(async function test_openTabs_mixedtype_input() {
// Passing the userContextId as a string, rather than a number, is a fairly
// common mistake, check the API handles both properly.
const url = "http://someurl.mozilla.org/";
Assert.deepEqual(
[],
UrlbarProviderOpenTabs.getOpenTabs(1),
"Found all the expected tabs"
);
Assert.deepEqual(
[],
UrlbarProviderOpenTabs.getOpenTabs(2),
"Found all the expected tabs"
);
UrlbarProviderOpenTabs.registerOpenTab(url, 1, false);
UrlbarProviderOpenTabs.registerOpenTab(url, "2", false);
Assert.deepEqual(
[url],
UrlbarProviderOpenTabs.getOpenTabs(1),
"Found all the expected tabs"
);
Assert.deepEqual(
[url],
UrlbarProviderOpenTabs.getOpenTabs(2),
"Found all the expected tabs"
);
Assert.deepEqual(
UrlbarProviderOpenTabs.getOpenTabs(1),
UrlbarProviderOpenTabs.getOpenTabs("1"),
"Also check getOpenTabs adapts to the argument type"
);
UrlbarProviderOpenTabs.unregisterOpenTab(url, "1", false);
UrlbarProviderOpenTabs.unregisterOpenTab(url, 2, false);
Assert.deepEqual(
[],
UrlbarProviderOpenTabs.getOpenTabs(1),
"Found all the expected tabs"
);
Assert.deepEqual(
[],
UrlbarProviderOpenTabs.getOpenTabs(2),
"Found all the expected tabs"
);
});