Skip to content

Commit

Permalink
Fix summarize concurrency (#30)
Browse files Browse the repository at this point in the history
* Fix summarize concurrency

This fixes the issue reported in https://kagifeedback.org/d/1978-firefox-extension-race-condition-when-summarizing-multiple-pages where many concurrent summarization requests could end up being "overwritten" by more recent requests.

It also fixes a different race condition when opening the summarization via shortcut.

* Fix favicon URL for search provider (Firefox demands a URL now, not a local path)
  • Loading branch information
BrunoBernardino authored Sep 24, 2023
1 parent d913212 commit 7fad301
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 11 deletions.
4 changes: 2 additions & 2 deletions chrome/manifest.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"manifest_version": 3,
"name": "Kagi Search for Chrome",
"version": "0.3.7",
"version": "0.3.8",
"description": "A simple extension for setting Kagi as a default search engine, and automatically logging in to Kagi in incognito browsing windows",
"background": {
"service_worker": "src/background.js",
Expand Down Expand Up @@ -30,7 +30,7 @@
"search_provider": {
"name": "Kagi",
"search_url": "https://kagi.com/search?q={searchTerms}",
"favicon_url": "https://kagi.com/favicon.ico",
"favicon_url": "https://assets.kagi.com/v2/favicon-32x32.png",
"keyword": "@kagi",
"is_default": true,
"suggest_url": "https://kagi.com/api/autosuggest?q={searchTerms}",
Expand Down
4 changes: 2 additions & 2 deletions firefox/manifest.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"manifest_version": 3,
"name": "Kagi Search for Firefox",
"version": "0.3.7",
"version": "0.3.8",
"description": "A simple helper extension for setting Kagi as a default search engine, and automatically logging in to Kagi in incognito browsing windows.",
"background": {
"page": "src/background_page.html"
Expand Down Expand Up @@ -29,7 +29,7 @@
"search_provider": {
"name": "Kagi",
"search_url": "https://kagi.com/search?q={searchTerms}",
"favicon_url": "icons/icon_32px.png",
"favicon_url": "https://assets.kagi.com/v2/favicon-32x32.png",
"keyword": "@kagi",
"is_default": true,
"suggest_url": "https://kagi.com/api/autosuggest?q={searchTerms}",
Expand Down
1 change: 1 addition & 0 deletions shared/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ async function summarizePage(options) {
type: 'summary_finished',
summary,
success,
url: options.url,
});
}
}
Expand Down
15 changes: 11 additions & 4 deletions shared/src/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,18 @@ export async function fetchSettings() {
};
}

export async function getActiveTab() {
const tabs = await browser.tabs.query({
export async function getActiveTab(fetchingFromShortcut = false) {
const tabsQuery = {
active: true,
lastFocusedWindow: true,
});
};

// Don't look just in the last focused window if we're opening from the shortcut
if (fetchingFromShortcut) {
tabsQuery.lastFocusedWindow = undefined;
}

const tabs = await browser.tabs.query(tabsQuery);

// Chrome/Firefox might give us more than one active tab when something like "chrome://*" or "about:*" is also open
const tab =
Expand All @@ -129,7 +136,7 @@ export async function getActiveTab() {

if (!tab || !tab.url) {
console.error('No tab/url found.');
console.error(tabs);
console.error(JSON.stringify(tabs));
return null;
}

Expand Down
14 changes: 11 additions & 3 deletions shared/src/summarize_result.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ async function setup() {
});

browser.runtime.onMessage.addListener(async (data) => {
if (data.type === 'summary_finished') {
const searchParams = new URLSearchParams(window.location.search);
const url = searchParams.get('url');

if (data.type === 'summary_finished' && data.url === url) {
loadingElement.style.display = 'none';

if (data.success) {
Expand Down Expand Up @@ -90,14 +93,19 @@ async function setup() {
searchParams.set('summary_type', 'summary');
searchParams.set('target_language', '');

const tab = await getActiveTab();
const tab = await getActiveTab(true);

if (!tab) {
console.error('No tab/url found.');
return;
}

searchParams.set('url', url);
searchParams.set('url', tab.url);

// Add ?url=<tab.url> to the window, so it receives the proper summary
const popupUrl = new URL(window.location.href);
popupUrl.searchParams.set('url', tab.url);
window.history.replaceState(null, '', popupUrl.toString());

const { token, api_token, api_engine } = await fetchSettings();

Expand Down

0 comments on commit 7fad301

Please sign in to comment.