Skip to content

Commit d51ede2

Browse files
kjkclaude
andcommitted
Sync Bookmarks panel selection when navigating via Command Palette (#5716)
Navigating to a bookmark through the Command Palette (the "*"/TOC mode) moved the page but left the Bookmarks (table-of-contents) panel selected on the previous item. GoToTocLink sets win->tocKeepSelection=true so the page-based UpdateTocSelection doesn't override a tree-driven selection. That's correct for panel clicks (the tree already selected the item), but the palette path never selects anything in the tree, so the selection stayed stale. Add a selectInTree flag (set only on the palette path) that, after navigating, programmatically selects and expands to the item -- which doesn't re-navigate, since TocTreeSelectionChanged only acts on byKeyboard/byMouse selection changes. Add an ad-hoc GUI-automation regression test (tests/ad-hoc-toc-palette-sync.ts, registered in before-release.ts) that drives the palette and asserts the tree selection moves; it fails when the fix is reverted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c0f5528 commit d51ede2

3 files changed

Lines changed: 150 additions & 0 deletions

File tree

‎src/TableOfContents.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ struct GoToTocLinkData {
176176
TocItem* tocItem;
177177
WindowTab* tab;
178178
DocController* ctrl;
179+
// true when the navigation was driven from outside the tree (e.g. the
180+
// command palette), so afterwards we must move the tree's selection to the
181+
// item ourselves. For tree-driven navigation the tree is already selected.
182+
bool selectInTree = false;
179183
};
180184

181185
static void GoToTocLink(GoToTocLinkData* d) {
@@ -207,6 +211,19 @@ static void GoToTocLink(GoToTocLinkData* d) {
207211
ctrl->GoToPage(pageNo, true);
208212
}
209213
win->tocKeepSelection = false;
214+
215+
// when driven from the command palette the tree wasn't the source of the
216+
// navigation, so the page-based UpdateTocSelection was suppressed above and
217+
// the tree still shows the old item. Move the selection to this item now
218+
// (programmatic SelectItem doesn't re-navigate -- see TocTreeSelectionChanged).
219+
if (d->selectInTree && win->tocLoaded && win->tocTreeView) {
220+
TreeView* treeView = win->tocTreeView;
221+
HTREEITEM hi = treeView->GetHandleByTreeItem((TreeItem)tocItem);
222+
if (hi) {
223+
TreeView_EnsureVisible(treeView->hwnd, hi);
224+
}
225+
treeView->SelectItem((TreeItem)tocItem);
226+
}
210227
}
211228

212229
// navigate to a TocItem regardless of whether it points to a page in this
@@ -220,6 +237,7 @@ void GoToTocItem(MainWindow* win, TocItem* tocItem) {
220237
data->ctrl = win->ctrl;
221238
data->tocItem = tocItem;
222239
data->tab = win->CurrentTab();
240+
data->selectInTree = true; // palette-driven: sync the tree selection too
223241
auto fn = MkFunc0<GoToTocLinkData>(GoToTocLink, data);
224242
uitask::Post(fn, "TaskGoToTocFromPalette");
225243
}

‎tests/ad-hoc-toc-palette-sync.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Ad-hoc regression test for issue #5716: navigating to a bookmark via the
2+
// Command Palette (the "*" / TOC mode) must move the selection in the Bookmarks
3+
// (table-of-contents) panel too. Before the fix the page navigated but the tree
4+
// selection stayed on the previous item.
5+
//
6+
// This is GUI automation (there's no -dbg-control hook for palette TOC
7+
// navigation), so it lives as an ad-hoc test, not in tests/all.ts. Run directly:
8+
// bun tests/ad-hoc-toc-palette-sync.ts [--no-build]
9+
//
10+
// Reverting the fix (the selectInTree path in TableOfContents.cpp's GoToTocLink)
11+
// makes the tree selection stay put, so this throws.
12+
13+
import { writeFileSync } from "node:fs";
14+
import { runStandalone, tmpPath } from "./util.ts";
15+
import { launchSumatra, waitForFrame, findChildByClass, sendCommand, pressEnter } from "./win-automation.ts";
16+
import {
17+
sleep,
18+
moveWindow,
19+
treeGetSelection,
20+
enumWindows,
21+
getWindowPid,
22+
findChildWindow,
23+
sendText,
24+
} from "./winapi.ts";
25+
26+
const CmdCommandPaletteTOC = 434;
27+
28+
// a 4-page PDF with 4 top-level bookmarks (one per page), /UseOutlines so the
29+
// bookmarks panel opens automatically
30+
function makeBookmarkedPdf(): Buffer {
31+
const enc = (s: string) => Buffer.from(s, "latin1");
32+
const titles = ["Chapter One", "Chapter Two", "Chapter Three", "Chapter Four"];
33+
const body: Record<number, Buffer> = {};
34+
body[1] = enc("<< /Type /Catalog /Pages 2 0 R /Outlines 8 0 R /PageMode /UseOutlines >>");
35+
body[2] = enc("<< /Type /Pages /Kids [3 0 R 4 0 R 5 0 R 6 0 R] /Count 4 >>");
36+
for (let i = 0; i < 4; i++) {
37+
const pageObj = 3 + i;
38+
const contentObj = 13 + i;
39+
const stream = `BT /F1 28 Tf 72 700 Td (Page ${i + 1} - ${titles[i]}) Tj ET`;
40+
body[contentObj] = enc(`<< /Length ${stream.length} >>\nstream\n${stream}\nendstream`);
41+
body[pageObj] = enc(
42+
`<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] ` +
43+
`/Resources << /Font << /F1 7 0 R >> >> /Contents ${contentObj} 0 R >>`,
44+
);
45+
}
46+
body[7] = enc("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>");
47+
body[8] = enc("<< /Type /Outlines /First 9 0 R /Last 12 0 R /Count 4 >>");
48+
for (let i = 0; i < 4; i++) {
49+
const itemObj = 9 + i;
50+
const prev = i > 0 ? ` /Prev ${itemObj - 1} 0 R` : "";
51+
const next = i < 3 ? ` /Next ${itemObj + 1} 0 R` : "";
52+
body[itemObj] = enc(`<< /Title (${titles[i]}) /Parent 8 0 R${prev}${next} /Dest [${3 + i} 0 R /Fit] >>`);
53+
}
54+
55+
const maxN = 16;
56+
const parts: Buffer[] = [enc("%PDF-1.7\n%\xe2\xe3\xcf\xd3\n")];
57+
const offsets: Record<number, number> = {};
58+
let pos = parts[0].length;
59+
for (let n = 1; n <= maxN; n++) {
60+
offsets[n] = pos;
61+
const obj = Buffer.concat([enc(`${n} 0 obj\n`), body[n], enc("\nendobj\n")]);
62+
parts.push(obj);
63+
pos += obj.length;
64+
}
65+
let xref = `xref\n0 ${maxN + 1}\n0000000000 65535 f \n`;
66+
for (let n = 1; n <= maxN; n++) {
67+
xref += `${String(offsets[n]).padStart(10, "0")} 00000 n \n`;
68+
}
69+
parts.push(enc(`${xref}trailer\n<< /Size ${maxN + 1} /Root 1 0 R >>\nstartxref\n${pos}\n%%EOF\n`));
70+
return Buffer.concat(parts);
71+
}
72+
73+
function topWindows(pid: number): number[] {
74+
const out: number[] = [];
75+
enumWindows((h) => {
76+
if (getWindowPid(h) === pid) {
77+
out.push(h);
78+
}
79+
return true;
80+
});
81+
return out;
82+
}
83+
84+
export async function testit(): Promise<void> {
85+
const pdfPath = tmpPath("issue-5716-bookmarks.pdf");
86+
writeFileSync(pdfPath, makeBookmarkedPdf());
87+
88+
const proc = launchSumatra([pdfPath]);
89+
try {
90+
const frame = await waitForFrame(proc.pid);
91+
if (!frame) {
92+
throw new Error("SumatraPDF frame window didn't appear");
93+
}
94+
moveWindow(frame, 40, 20, 1000, 760);
95+
await sleep(1200);
96+
97+
const tree = findChildByClass(frame, "SysTreeView32");
98+
if (!tree) {
99+
throw new Error("Bookmarks (TOC) tree not found - did the outline load?");
100+
}
101+
const sel0 = treeGetSelection(tree); // Chapter One (page 1)
102+
103+
// open the palette in TOC ("*") mode, filter to Chapter Four, navigate to it
104+
const before = new Set(topWindows(proc.pid));
105+
sendCommand(frame, CmdCommandPaletteTOC);
106+
await sleep(700);
107+
const palette = topWindows(proc.pid).find((h) => !before.has(h) && findChildWindow(h, "Edit")) ?? 0;
108+
if (!palette) {
109+
throw new Error("Command Palette window didn't open");
110+
}
111+
const edit = findChildWindow(palette, "Edit");
112+
sendText(edit, "*Four"); // keep the "*" TOC prefix; filter to "Chapter Four"
113+
await sleep(400);
114+
await pressEnter(edit); // navigate to the selected (Chapter Four) bookmark
115+
await sleep(600);
116+
117+
const sel1 = treeGetSelection(tree);
118+
if (sel1 === sel0) {
119+
throw new Error(
120+
"issue #5716: Bookmarks panel selection did not move after navigating via the Command Palette",
121+
);
122+
}
123+
} finally {
124+
proc.kill();
125+
}
126+
}
127+
128+
if (import.meta.main) {
129+
await runStandalone(testit);
130+
}

‎tests/before-release.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@ import { testit as allTests } from "./all.ts";
1010
import { testit as adHocExif } from "./ad-hoc-exif.ts";
1111
import { testit as adHocSelectionTranslate } from "./ad-hoc-selection-translate.ts";
1212
import { testit as adHocTripleClickLine } from "./ad-hoc-triple-click-line.ts";
13+
import { testit as adHocTocPaletteSync } from "./ad-hoc-toc-palette-sync.ts";
1314
import { testit as issueChmLzx } from "./issue-chm-lzx.ts";
1415

1516
const adHocTests: [string, () => void | Promise<void>][] = [
1617
["ad-hoc-exif", adHocExif],
1718
["ad-hoc-selection-translate", adHocSelectionTranslate],
1819
["ad-hoc-triple-click-line", adHocTripleClickLine],
20+
["ad-hoc-toc-palette-sync", adHocTocPaletteSync],
1921
["issue-chm-lzx", issueChmLzx],
2022
];
2123

0 commit comments

Comments
 (0)