[MV2] Temporary fix for the unique tab feature
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
diff --git a/src/background.ts b/src/background.ts
index b0af7de..2db72b9 100644
--- a/src/background.ts
+++ b/src/background.ts
@@ -93,7 +93,7 @@
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});
});
}
})
diff --git a/src/common/sessionStorage_mv2.ts b/src/common/sessionStorage_mv2.ts
index dda0ae1..55990f3 100644
--- a/src/common/sessionStorage_mv2.ts
+++ b/src/common/sessionStorage_mv2.ts
@@ -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);
+ });
});
}
}