Skip to content

Commit

Permalink
[MV2] Temporary fix for the unique tab feature
Browse files Browse the repository at this point in the history
The unique tab feature doesn't work correctly in the MV2 variant of the
extension.

This CL fixes this by changing how the data managed by the
sessionStorage class is stored in the MV2 variant:

- Before, it was saved in the window object, which was erased each time
  the background script was turned inactive by the browser. This
  behavior was fine for most data, but not for the "translatorTab" data,
  which is why there was this bug.
- Now, it's saved in the persisted "local" storage area. For more
  information about this behavior, read the comment added to the
  //src/common/sessionStorage_mv2.js file.

Fixed: twpowertools:18
Change-Id: Ie3979937f4bd0699b57506666beace3ffe00172a
  • Loading branch information
avm99963 committed Nov 14, 2022
1 parent fd18d73 commit 9ff52fe
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function translationClick(
chrome.windows.create({type: 'popup', url, width: 1000, height: 382});
} else {
chrome.tabs.create(newTabOptions, tab => {
ExtSessionStorage.set({translatorTab: tab.id});
ExtSessionStorage.set({translatorTab: tab?.id});
});
}
})
Expand Down
79 changes: 41 additions & 38 deletions src/common/sessionStorage_mv2.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,50 @@
declare global {
interface Window {
extCustomStorage: any;
}
}
// WARNING: In MV2, data is persisted even after the session ends,[1] unlike
// with chrome.storage.session in MV3.
//
// Before, this class in MV2 only persisted data until the background page was
// unloaded, so this introduced bug
// https://iavm.xyz/b/translateselectedtext/18.
//
// Thus, since making the background page persistent will impact performance,
// the easiest solution is to let this custom session storage be persistent in
// MV2, because persistence of this storage won't affect the extension, and
// although this solution isn't nice and could introduce some smaller bugs, MV2
// will be deprecated soon anyways.
//
// [1]: The only exception is the translatorTab value, which will be erased when
// the corresponding tab is closed. This is because after a browser restart,
// already used tab IDs can be reused again, so we could end up opening Google
// Translate in an unrelated tab otherwise.
//
// Also, we'll delete this value when starting the browser as another
// "protection layer", since closing the browser sometimes results in the
// onRemoved event not being handled.
//
// Keep in mind there are some edge cases where we could still be reusing an
// unrelated tab. A known one is when the extension is disabled for some period
// of time and afterwards enabled again, but there might be others.

chrome.runtime.onStartup.addListener(() => {
chrome.storage.local.remove('translatorTab');
});

chrome.tabs.onRemoved.addListener(tabId => {
ExtSessionStorage.get('translatorTab').then(items => {
if (tabId === items['translatorTab'])
chrome.storage.local.remove('translatorTab');
});
});

export default class ExtSessionStorage {
static set(items: any): Promise<void> {
return new Promise((res, rej) => {
if (window.extCustomStorage === undefined) window.extCustomStorage = {};

for (const [key, value] of Object.entries(items))
window.extCustomStorage[key] = value;

res();
});
return chrome.storage.local.set(items);
}

static get(keys: string|Array<string>|undefined): Promise<any> {
return new Promise((res, rej) => {
if (window.extCustomStorage === undefined) window.extCustomStorage = {};

if (keys === undefined) {
res(window.extCustomStorage);
return;
}

if (typeof keys === 'string') {
const key = keys;
keys = [key];
}

if (Array.isArray(keys)) {
let returnObject: any = {};
for (const key of keys) {
returnObject[key] = window.extCustomStorage[key];
}
res(returnObject);
return;
}

rej(new Error(
'The keys passed are not a valid type ' +
'(undefined, string or array).'));
return new Promise((res, _) => {
chrome.storage.local.get(keys, items => {
res(items);
});
});
}
}

0 comments on commit 9ff52fe

Please sign in to comment.