Skip to content

Commit 77e7ad4

Browse files
authored
395 context menu item translate page is sometimes not displayed (#567)
# Problem We have single context menu for all tabs and dynamically update it when user toggle/refresh tab. While this update we fetch content script for state of page translation and sometimes it happens too quickly, so content script is not ready to response for requests. This lead us to an exception and `catch` block is hide context menu in result. # How this change fixes it Implemented retries mechanism for query content script. Now background script can wait response (with reasonable deadline time). --- * fix(#395): wait response of content script * chore: update debug print
1 parent 28b3116 commit 77e7ad4

File tree

3 files changed

+48
-12
lines changed

3 files changed

+48
-12
lines changed

src/app/ContextMenus/TranslatePageContextMenu.ts

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import browser from 'webextension-polyfill';
44
import { isFirefox } from '../../lib/browser';
55
import { getCurrentTabId, isValidBrowserTabId } from '../../lib/browser/tabs';
66
import { getMessage } from '../../lib/language';
7+
import { wait } from '../../lib/time';
78
import { getConfig } from '../../requests/backend/getConfig';
89
import { getTranslatorFeatures } from '../../requests/backend/getTranslatorFeatures';
910
import { disableTranslatePage } from '../../requests/contentscript/pageTranslation/disableTranslatePage';
@@ -96,7 +97,12 @@ export class TranslatePageContextMenu {
9697
}
9798
}
9899

100+
private updateMenuContext: unknown = null;
99101
private readonly updateMenuItem = async (tabId: number) => {
102+
// Update context
103+
const currentContext = {};
104+
this.updateMenuContext = currentContext;
105+
100106
const currentWindow = await browser.windows.getCurrent();
101107
const tab = await browser.tabs.get(tabId);
102108

@@ -123,18 +129,46 @@ export class TranslatePageContextMenu {
123129
});
124130

125131
if (isVisible) {
126-
try {
127-
const translateState = await getPageTranslateState(tabId);
128-
this.tabStateUpdated({
129-
tabId,
130-
isTranslating: translateState.isTranslated,
131-
});
132-
} catch (_error) {
133-
// Handle case when tab contentscript is not loaded yet
134-
// and requests do not handle
135-
browser.contextMenus.update(this.menuId, {
136-
visible: false,
137-
});
132+
const timeout = 50;
133+
const retriesLimit = 30;
134+
135+
// Retry loop
136+
// eslint-disable-next-line no-constant-condition
137+
for (let retry = 0; true; retry++) {
138+
// Handle case if user have changed tab while waiting
139+
if (this.updateMenuContext !== currentContext) break;
140+
141+
try {
142+
const translateState = await getPageTranslateState(tabId);
143+
144+
browser.contextMenus.update(this.menuId, {
145+
visible: true,
146+
});
147+
148+
this.tabStateUpdated({
149+
tabId,
150+
isTranslating: translateState.isTranslated,
151+
});
152+
153+
break;
154+
} catch (error) {
155+
// Handle case when tab contentscript is not loaded yet
156+
// and requests do not handle
157+
browser.contextMenus.update(this.menuId, {
158+
visible: false,
159+
});
160+
161+
// Retry attempt
162+
if (retry < retriesLimit) {
163+
await wait(timeout);
164+
continue;
165+
}
166+
167+
// Out of limit
168+
console.warn('Error while render context menu for page translation');
169+
console.error(error);
170+
break;
171+
}
138172
}
139173
}
140174
};

src/lib/time.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const wait = (time: number) => new Promise((res) => setTimeout(res, time));

webpack.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ console.log('WebpackConfig', {
3838
mode,
3939
target,
4040
outputPath,
41+
isFastBuild,
4142
});
4243

4344
const offscreenDocuments = ['main', 'worker', 'translator'];

0 commit comments

Comments
 (0)