Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix buggy behavior when changing the title of a bookmark manually #254

Merged
merged 1 commit into from
Dec 6, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/background/setup/initBackgroundScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { sendRequestToContent } from "../messaging/sendRequestToContent";
import { createContextMenus } from "../misc/createContextMenus";
import { initTabMarkers } from "../misc/tabMarkers";
import { setBrowserActionIcon } from "../utils/browserAction";
import { getCurrentTab } from "../utils/getCurrentTab";
import { isSafari } from "../utils/isSafari";
import { trackRecentTabs } from "./trackRecentTabs";

Expand Down Expand Up @@ -68,10 +69,24 @@ browser.runtime.onStartup.addListener(async () => {
/**
* Strip the tab marker and url added by Rango from the title of the bookmark.
*/
async function resetBookmarkTitle(id: string) {
async function resetBookmarkTitle(
id: string,
changeInfo: browser.Bookmarks.OnChangedChangeInfoType
) {
const includeTabMarkers = await retrieve("includeTabMarkers");
const urlInTitle = await retrieve("urlInTitle");

const currentTab = await getCurrentTab();
const { title: bookmarkTitle, url: bookmarkUrl } = changeInfo;

if (
!bookmarkUrl ||
currentTab.url !== bookmarkUrl ||
currentTab.title !== bookmarkTitle
) {
return;
}

if (includeTabMarkers || urlInTitle) {
try {
const titleBeforeDecorations = (await sendRequestToContent({
Expand All @@ -86,15 +101,19 @@ async function resetBookmarkTitle(id: string) {
browser.bookmarks?.onChanged.addListener(resetBookmarkTitle);
}
} catch {
// Do nothing. The user might be modifying the bookmark manually. In that
// case sendRequestToContent would fail.
// Do nothing. The user might be adding a bookmark to a page where the
// content script can't run. In that case the title wouldn't have been
// changed.
}
}
}

// We use optional chaining because this isn't supported in Safari.
browser.bookmarks?.onCreated.addListener(resetBookmarkTitle);

// In Chrome the bookmark is created when we open the dialog and updated when we
// submit.
// We need to add a listener to onChanged here because of the way bookmarks are
// saved in Chrome. When the bookmark popup appears the bookmark is saved. We
// change the title after onCreated is triggered, but when the user hits "done"
// the title of the bookmark will be changed again to the value of the input
// field of the popup window.
browser.bookmarks?.onChanged.addListener(resetBookmarkTitle);