Skip to content

Commit 9b60f8d

Browse files
ralyodioralyodio
committed
fix: cross-browser bookmark ordering sync
Three fixes for bookmark order not syncing between Firefox and Chromium: 1. Don't track sync-driven moves/creates as locally modified - Added isSyncDrivenChange flag, set true during addCloudBookmarksToLocal and updateLocalBookmarksFromCloud - onMoved/onCreated/onChanged listeners skip locallyModifiedBookmarkIds tracking when flag is true - Prevents stale IDs from causing categorizeCloudBookmarks to skip cloud index updates on subsequent syncs 2. Cancel debounced save when clearing modified IDs at end of sync - clearTimeout(saveModifiedIdsTimeout) before clear() + save - Prevents race condition where async onMoved handler's debounced save fires AFTER the clear, re-persisting stale IDs to storage 3. Normalize folder paths in checksum calculation (both extension + server) - 'Bookmarks Bar' and 'Bookmarks Toolbar' now both normalize to 'toolbar/' - Prevents eternal checksum mismatch between Firefox and Chrome - Eliminates unnecessary push-after-pull cycles that could overwrite correct ordering
1 parent 1529e63 commit 9b60f8d

11 files changed

Lines changed: 140 additions & 75 deletions

File tree

‎apps/extension/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@marksyncr/extension",
3-
"version": "0.8.16",
3+
"version": "0.8.17",
44
"private": true,
55
"type": "module",
66
"scripts": {

‎apps/extension/src/background/index.js‎

Lines changed: 114 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ let pendingSyncReasons = [];
3333
// Track bookmarks modified locally since last sync - these take priority over cloud versions
3434
let locallyModifiedBookmarkIds = new Set();
3535

36+
// Flag to suppress locallyModifiedBookmarkIds tracking during sync-driven moves/creates
37+
// When true, bookmark events from updateLocalBookmarksFromCloud / addCloudBookmarksToLocal
38+
// are NOT tracked as "locally modified" because they are cloud-driven, not user-driven.
39+
let isSyncDrivenChange = false;
40+
3641
// Retry limiting for failed syncs
3742
const MAX_CONSECUTIVE_FAILURES = 3;
3843
let consecutiveSyncFailures = 0;
@@ -209,12 +214,27 @@ function normalizeItemsForChecksum(items) {
209214

210215
return items
211216
.map((item) => {
217+
// Normalize folder path for cross-browser consistency in checksum
218+
// Without this, Firefox ("Bookmarks Toolbar") and Chrome ("Bookmarks Bar")
219+
// always produce different checksums, causing unnecessary push-after-pull cycles
220+
const rawPath = item.folderPath || item.folder_path || '';
221+
const normalizedPath = rawPath
222+
.replace(/^Bookmarks Bar\/?/i, 'toolbar/')
223+
.replace(/^Bookmarks Toolbar\/?/i, 'toolbar/')
224+
.replace(/^Speed Dial\/?/i, 'toolbar/')
225+
.replace(/^Favourites Bar\/?/i, 'toolbar/')
226+
.replace(/^Favorites Bar\/?/i, 'toolbar/')
227+
.replace(/^Other Bookmarks\/?/i, 'other/')
228+
.replace(/^Unsorted Bookmarks\/?/i, 'other/')
229+
.replace(/^Bookmarks Menu\/?/i, 'menu/')
230+
.replace(/\/+$/, '');
231+
212232
if (item.type === 'folder') {
213233
// Folder entry
214234
return {
215235
type: 'folder',
216236
title: item.title ?? '',
217-
folderPath: item.folderPath || item.folder_path || '',
237+
folderPath: normalizedPath,
218238
index: item.index ?? 0,
219239
};
220240
} else {
@@ -224,7 +244,7 @@ function normalizeItemsForChecksum(items) {
224244
type: 'bookmark',
225245
url: item.url,
226246
title: item.title ?? '',
227-
folderPath: item.folderPath || item.folder_path || '',
247+
folderPath: normalizedPath,
228248
index: item.index ?? 0,
229249
};
230250
}
@@ -1226,9 +1246,12 @@ function setupBookmarkListeners() {
12261246
browser.bookmarks.onCreated.addListener(async (id, bookmark) => {
12271247
console.log('[MarkSyncr] Bookmark created:', bookmark.title);
12281248

1229-
// Always track locally modified bookmarks, even during sync
1230-
locallyModifiedBookmarkIds.add(id);
1231-
debouncedSaveLocallyModifiedIds();
1249+
// Don't track sync-driven creates (from addCloudBookmarksToLocal) as locally modified
1250+
// Only track user-initiated bookmark creates
1251+
if (!isSyncDrivenChange) {
1252+
locallyModifiedBookmarkIds.add(id);
1253+
debouncedSaveLocallyModifiedIds();
1254+
}
12321255

12331256
// Skip processing during sync operations to prevent sync loops
12341257
// BUT mark that we need a follow-up sync after current sync completes
@@ -1289,9 +1312,11 @@ function setupBookmarkListeners() {
12891312
browser.bookmarks.onChanged.addListener((id, changeInfo) => {
12901313
console.log('[MarkSyncr] Bookmark changed:', id);
12911314

1292-
// Always track locally modified bookmarks, even during sync
1293-
locallyModifiedBookmarkIds.add(id);
1294-
debouncedSaveLocallyModifiedIds();
1315+
// Don't track sync-driven changes as locally modified
1316+
if (!isSyncDrivenChange) {
1317+
locallyModifiedBookmarkIds.add(id);
1318+
debouncedSaveLocallyModifiedIds();
1319+
}
12951320

12961321
// Skip during sync operations to prevent sync loops
12971322
// BUT mark that we need a follow-up sync after current sync completes
@@ -1309,57 +1334,63 @@ function setupBookmarkListeners() {
13091334
browser.bookmarks.onMoved.addListener(async (id, moveInfo) => {
13101335
console.log('[MarkSyncr] Bookmark moved:', id);
13111336

1312-
// Always track the moved bookmark itself
1313-
locallyModifiedBookmarkIds.add(id);
1314-
1315-
// CRITICAL: When a bookmark is moved, all siblings in the affected folder(s)
1316-
// have their indices shifted implicitly by the browser. We must mark them all
1317-
// as locally modified so the sync doesn't override their new positions with
1318-
// stale cloud indices. Without this, reordering a single bookmark causes the
1319-
// cloud's old order to be re-applied to all the un-tracked siblings.
1320-
try {
1321-
const foldersToMark = new Set([moveInfo.parentId]);
1322-
if (moveInfo.oldParentId && moveInfo.oldParentId !== moveInfo.parentId) {
1323-
foldersToMark.add(moveInfo.oldParentId);
1324-
}
1325-
for (const folderId of foldersToMark) {
1326-
const children = await browser.bookmarks.getChildren(folderId);
1327-
for (const child of children) {
1328-
locallyModifiedBookmarkIds.add(child.id);
1337+
// Don't track sync-driven moves (from updateLocalBookmarksFromCloud) as locally modified.
1338+
// These are cloud-driven changes, not user-driven. Tracking them causes stale IDs to
1339+
// persist (via debounced save race condition), which makes the next sync skip cloud
1340+
// updates — breaking cross-browser order sync.
1341+
if (!isSyncDrivenChange) {
1342+
// Track the moved bookmark itself
1343+
locallyModifiedBookmarkIds.add(id);
1344+
1345+
// CRITICAL: When a bookmark is moved, all siblings in the affected folder(s)
1346+
// have their indices shifted implicitly by the browser. We must mark them all
1347+
// as locally modified so the sync doesn't override their new positions with
1348+
// stale cloud indices. Without this, reordering a single bookmark causes the
1349+
// cloud's old order to be re-applied to all the un-tracked siblings.
1350+
try {
1351+
const foldersToMark = new Set([moveInfo.parentId]);
1352+
if (moveInfo.oldParentId && moveInfo.oldParentId !== moveInfo.parentId) {
1353+
foldersToMark.add(moveInfo.oldParentId);
13291354
}
1330-
}
1331-
console.log(
1332-
`[MarkSyncr] Tracked ${foldersToMark.size} folder(s) with all siblings as locally modified`
1333-
);
1355+
for (const folderId of foldersToMark) {
1356+
const children = await browser.bookmarks.getChildren(folderId);
1357+
for (const child of children) {
1358+
locallyModifiedBookmarkIds.add(child.id);
1359+
}
1360+
}
1361+
console.log(
1362+
`[MarkSyncr] Tracked ${foldersToMark.size} folder(s) with all siblings as locally modified`
1363+
);
13341364

1335-
// CRITICAL: When a FOLDER is moved, the bookmarks inside it change their
1336-
// effective folderPath, but don't fire individual onMoved events. We must
1337-
// recursively mark all descendants as locally modified so the sync doesn't
1338-
// move them back to the cloud's old folder path.
1339-
const movedNodes = await browser.bookmarks.getSubTree(id);
1340-
if (movedNodes[0]?.children) {
1341-
let descendantCount = 0;
1342-
const markDescendants = (nodes) => {
1343-
for (const node of nodes) {
1344-
locallyModifiedBookmarkIds.add(node.id);
1345-
descendantCount++;
1346-
if (node.children) {
1347-
markDescendants(node.children);
1365+
// CRITICAL: When a FOLDER is moved, the bookmarks inside it change their
1366+
// effective folderPath, but don't fire individual onMoved events. We must
1367+
// recursively mark all descendants as locally modified so the sync doesn't
1368+
// move them back to the cloud's old folder path.
1369+
const movedNodes = await browser.bookmarks.getSubTree(id);
1370+
if (movedNodes[0]?.children) {
1371+
let descendantCount = 0;
1372+
const markDescendants = (nodes) => {
1373+
for (const node of nodes) {
1374+
locallyModifiedBookmarkIds.add(node.id);
1375+
descendantCount++;
1376+
if (node.children) {
1377+
markDescendants(node.children);
1378+
}
13481379
}
1380+
};
1381+
markDescendants(movedNodes[0].children);
1382+
if (descendantCount > 0) {
1383+
console.log(
1384+
`[MarkSyncr] Tracked ${descendantCount} descendants of moved folder as locally modified`
1385+
);
13491386
}
1350-
};
1351-
markDescendants(movedNodes[0].children);
1352-
if (descendantCount > 0) {
1353-
console.log(
1354-
`[MarkSyncr] Tracked ${descendantCount} descendants of moved folder as locally modified`
1355-
);
13561387
}
1388+
} catch (err) {
1389+
console.warn('[MarkSyncr] Failed to mark siblings as modified:', err.message);
13571390
}
1358-
} catch (err) {
1359-
console.warn('[MarkSyncr] Failed to mark siblings as modified:', err.message);
1360-
}
13611391

1362-
debouncedSaveLocallyModifiedIds();
1392+
debouncedSaveLocallyModifiedIds();
1393+
}
13631394

13641395
// Skip during sync operations to prevent sync loops
13651396
// BUT mark that we need a follow-up sync after current sync completes
@@ -1658,19 +1689,26 @@ async function performSync(sourceId) {
16581689
console.log(`[MarkSyncr] Local additions (not in cloud): ${localAdditions.length}`);
16591690

16601691
// Step 7: Add new cloud bookmarks to local browser
1661-
if (newFromCloud.length > 0) {
1662-
await addCloudBookmarksToLocal(newFromCloud);
1663-
console.log(`[MarkSyncr] Added ${newFromCloud.length} bookmarks from cloud to local`);
1664-
}
1692+
// Set isSyncDrivenChange so onCreated/onMoved listeners don't pollute
1693+
// locallyModifiedBookmarkIds with sync-driven changes
1694+
isSyncDrivenChange = true;
1695+
try {
1696+
if (newFromCloud.length > 0) {
1697+
await addCloudBookmarksToLocal(newFromCloud);
1698+
console.log(`[MarkSyncr] Added ${newFromCloud.length} bookmarks from cloud to local`);
1699+
}
16651700

1666-
// Step 7.5: Apply updates from cloud to local bookmarks
1667-
// This is critical for proper two-way sync: if another browser modified
1668-
// a bookmark's title, folder, or position, those changes should be pulled here.
1669-
let updatedLocally = 0;
1670-
if (bookmarksToUpdate.length > 0) {
1671-
console.log(`[MarkSyncr] Found ${bookmarksToUpdate.length} bookmarks to update from cloud`);
1672-
updatedLocally = await updateLocalBookmarksFromCloud(bookmarksToUpdate);
1673-
console.log(`[MarkSyncr] Updated ${updatedLocally} local bookmarks from cloud`);
1701+
// Step 7.5: Apply updates from cloud to local bookmarks
1702+
// This is critical for proper two-way sync: if another browser modified
1703+
// a bookmark's title, folder, or position, those changes should be pulled here.
1704+
let updatedLocally = 0;
1705+
if (bookmarksToUpdate.length > 0) {
1706+
console.log(`[MarkSyncr] Found ${bookmarksToUpdate.length} bookmarks to update from cloud`);
1707+
updatedLocally = await updateLocalBookmarksFromCloud(bookmarksToUpdate);
1708+
console.log(`[MarkSyncr] Updated ${updatedLocally} local bookmarks from cloud`);
1709+
}
1710+
} finally {
1711+
isSyncDrivenChange = false;
16741712
}
16751713

16761714
// Step 8: Get final local bookmarks after all merges
@@ -1725,6 +1763,12 @@ async function performSync(sourceId) {
17251763
lastSyncError = null;
17261764

17271765
// Clear locally modified tracking - changes are in sync
1766+
// Cancel any pending debounced save to prevent race condition where
1767+
// sync-driven onMoved events persist stale IDs after this clear
1768+
if (saveModifiedIdsTimeout) {
1769+
clearTimeout(saveModifiedIdsTimeout);
1770+
saveModifiedIdsTimeout = null;
1771+
}
17281772
locallyModifiedBookmarkIds.clear();
17291773
await saveLocallyModifiedIds();
17301774

@@ -1799,6 +1843,12 @@ async function performSync(sourceId) {
17991843
lastSyncError = null;
18001844

18011845
// Clear locally modified tracking - changes have been pushed to cloud
1846+
// Cancel any pending debounced save to prevent race condition where
1847+
// sync-driven onMoved events persist stale IDs after this clear
1848+
if (saveModifiedIdsTimeout) {
1849+
clearTimeout(saveModifiedIdsTimeout);
1850+
saveModifiedIdsTimeout = null;
1851+
}
18021852
locallyModifiedBookmarkIds.clear();
18031853
await saveLocallyModifiedIds();
18041854

‎apps/extension/src/manifest.chrome.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"manifest_version": 3,
33
"name": "MarkSyncr",
4-
"version": "0.8.16",
4+
"version": "0.8.17",
55
"description": "Sync your bookmarks across browsers using GitHub, Dropbox, Google Drive, or MarkSyncr Cloud",
66
"icons": {
77
"16": "icons/favicon-16.png",

‎apps/extension/src/manifest.firefox.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"manifest_version": 3,
33
"name": "MarkSyncr",
4-
"version": "0.8.16",
4+
"version": "0.8.17",
55
"description": "Sync your bookmarks across browsers using GitHub, Dropbox, Google Drive, or MarkSyncr Cloud",
66
"icons": {
77
"16": "icons/favicon-16.png",

‎apps/extension/src/manifest.safari.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"manifest_version": 3,
33
"name": "MarkSyncr",
4-
"version": "0.8.16",
4+
"version": "0.8.17",
55
"description": "Sync your bookmarks across browsers using GitHub, Dropbox, Google Drive, or MarkSyncr Cloud",
66
"icons": {
77
"16": "icons/favicon-16.png",

‎apps/web/app/api/bookmarks/route.js‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,27 @@ function normalizeItemsForChecksum(items) {
7676

7777
return items
7878
.map((item) => {
79+
// Normalize folder path for cross-browser consistency in checksum
80+
// Without this, Firefox ("Bookmarks Toolbar") and Chrome ("Bookmarks Bar")
81+
// always produce different checksums, causing unnecessary push-after-pull cycles
82+
const rawPath = item.folderPath || item.folder_path || '';
83+
const normalizedPath = rawPath
84+
.replace(/^Bookmarks Bar\/?/i, 'toolbar/')
85+
.replace(/^Bookmarks Toolbar\/?/i, 'toolbar/')
86+
.replace(/^Speed Dial\/?/i, 'toolbar/')
87+
.replace(/^Favourites Bar\/?/i, 'toolbar/')
88+
.replace(/^Favorites Bar\/?/i, 'toolbar/')
89+
.replace(/^Other Bookmarks\/?/i, 'other/')
90+
.replace(/^Unsorted Bookmarks\/?/i, 'other/')
91+
.replace(/^Bookmarks Menu\/?/i, 'menu/')
92+
.replace(/\/+$/, '');
93+
7994
if (item.type === 'folder') {
8095
// Folder entry
8196
return {
8297
type: 'folder',
8398
title: item.title ?? '',
84-
folderPath: item.folderPath || item.folder_path || '',
99+
folderPath: normalizedPath,
85100
index: item.index ?? 0,
86101
};
87102
} else {
@@ -91,7 +106,7 @@ function normalizeItemsForChecksum(items) {
91106
type: 'bookmark',
92107
url: item.url,
93108
title: item.title ?? '',
94-
folderPath: item.folderPath || item.folder_path || '',
109+
folderPath: normalizedPath,
95110
index: item.index ?? 0,
96111
};
97112
}

‎apps/web/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@marksyncr/web",
3-
"version": "0.8.16",
3+
"version": "0.8.17",
44
"private": true,
55
"type": "module",
66
"scripts": {

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "marksyncr",
3-
"version": "0.8.16",
3+
"version": "0.8.17",
44
"private": true,
55
"description": "Cross-browser bookmark sync extension with cloud storage",
66
"type": "module",

‎packages/core/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@marksyncr/core",
3-
"version": "0.8.16",
3+
"version": "0.8.17",
44
"private": true,
55
"type": "module",
66
"main": "./src/index.js",

‎packages/sources/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@marksyncr/sources",
3-
"version": "0.8.16",
3+
"version": "0.8.17",
44
"private": true,
55
"type": "module",
66
"main": "./src/index.js",

0 commit comments

Comments
 (0)