Skip to content

Commit f17604d

Browse files
PDF viewer - fix various rendering bugs (#1332)
- Fix mismatch between text position and highlight when in higher zoom levels - Fix case where contextual toolbar would close almost immediately after opening - Fix initialization error when accessing the pdf viewer URL directly
1 parent 2f06d47 commit f17604d

File tree

8 files changed

+352
-43
lines changed

8 files changed

+352
-43
lines changed

ts/packages/agents/browser/src/extension/contentScript/pdfInterceptor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export class PDFInterceptor {
6464
private async performConnectionCheck(): Promise<boolean> {
6565
try {
6666
const response = await chrome.runtime.sendMessage({
67-
action: "checkWebSocketConnection",
67+
type: "checkWebSocketConnection",
6868
});
6969
this.isWebSocketConnected = response?.connected || false;
7070
debug(`WebSocket connection status: ${this.isWebSocketConnected}`);

ts/packages/agents/browser/src/extension/contentScript/recording/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export function observeDOMChanges(): void {
126126

127127
// Optional: Send message to background script
128128
chrome.runtime.sendMessage({
129-
action: "spaNavigationDetected",
129+
type: "spaNavigationDetected",
130130
url: currentUrl,
131131
});
132132

ts/packages/agents/browser/src/views/client/pdf/app.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,9 @@ export class TypeAgentPDFViewerApp {
284284
): void {
285285
console.log("🎯 Highlight clicked:", highlightId);
286286

287+
// Prevent selection manager from closing toolbar immediately after we open it
288+
this.selectionManager?.ignoreSelectionChangesFor(300);
289+
287290
// Create a fake selection for the highlight area
288291
const fakeSelection: SelectionInfo = {
289292
text: highlightData.text || "Highlighted text",
@@ -799,8 +802,15 @@ export class TypeAgentPDFViewerApp {
799802
const toolbar = document.querySelector(".toolbar") as HTMLElement;
800803
const toolbarHeight = toolbar ? toolbar.offsetHeight : 56;
801804

805+
// Set container positioning - PDF.js requires absolute positioning
802806
viewerContainer.style.cssText = `
803-
position: relative; top: 0; left: 0; right: 0; height: calc(100vh - ${toolbarHeight}px); overflow: auto;
807+
position: absolute;
808+
top: ${toolbar ? toolbarHeight : 0}px;
809+
left: 0;
810+
right: 0;
811+
bottom: 0;
812+
overflow: auto;
813+
background: #323639;
804814
`;
805815

806816
const linkService = new window.pdfjsViewer.PDFLinkService({

ts/packages/agents/browser/src/views/client/pdf/core/pdfJSHighlightManager.ts

Lines changed: 92 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export class PDFJSHighlightManager {
8787
coordinates,
8888
pageNumber: selection.pageNumber,
8989
text: selection.text,
90-
creationScale: this.pdfViewer.currentScale || 1, // Store scale at creation time
90+
coordinateScale: coordinates.coordinateScale, // Store the scale at which coordinates were calculated
9191
};
9292

9393
// Store highlight
@@ -169,7 +169,7 @@ export class PDFJSHighlightManager {
169169
}
170170

171171
/**
172-
* Convert selection to coordinates using PDF.js viewport
172+
* Convert selection to coordinates using PDF.js text layer
173173
*/
174174
private convertSelectionToCoordinates(
175175
selection: SelectionInfo,
@@ -186,52 +186,73 @@ export class PDFJSHighlightManager {
186186
return null;
187187
}
188188

189-
const viewport = pageView.viewport;
190189
const pageElement = pageView.div;
191-
192190
if (!pageElement) {
193191
console.error("Page element not found");
194192
return null;
195193
}
196194

197-
const pageRect = pageElement.getBoundingClientRect();
195+
// Find the text layer within the page
196+
const textLayer = pageElement.querySelector(".textLayer");
197+
if (!textLayer) {
198+
console.error(
199+
"Text layer not found for page:",
200+
selection.pageNumber,
201+
);
202+
return null;
203+
}
204+
205+
const textLayerRect = textLayer.getBoundingClientRect();
206+
const currentScale = this.pdfViewer.currentScale || 1;
198207

199-
// Calculate bounds from selection rectangles relative to page
208+
console.log("🔍 Text layer rect:", textLayerRect);
209+
console.log("🔍 Current scale:", currentScale);
210+
211+
// Calculate bounds from selection rectangles relative to text layer
200212
let minLeft = Infinity;
201213
let maxRight = -Infinity;
202214
let minTop = Infinity;
203215
let maxBottom = -Infinity;
204216

205217
for (const rect of selection.rects) {
206-
const relativeLeft = rect.left - pageRect.left;
207-
const relativeTop = rect.top - pageRect.top;
208-
const relativeRight = rect.right - pageRect.left;
209-
const relativeBottom = rect.bottom - pageRect.top;
218+
console.log("🔍 Selection rect:", rect);
219+
220+
// Calculate position relative to text layer
221+
const relativeLeft = rect.left - textLayerRect.left;
222+
const relativeTop = rect.top - textLayerRect.top;
223+
const relativeRight = rect.right - textLayerRect.left;
224+
const relativeBottom = rect.bottom - textLayerRect.top;
210225

211226
minLeft = Math.min(minLeft, relativeLeft);
212227
maxRight = Math.max(maxRight, relativeRight);
213228
minTop = Math.min(minTop, relativeTop);
214229
maxBottom = Math.max(maxBottom, relativeBottom);
215230
}
216231

217-
return {
232+
const coordinates = {
218233
x: minLeft,
219234
y: minTop,
220235
width: maxRight - minLeft,
221236
height: maxBottom - minTop,
222237
pageRect: {
223-
width: pageRect.width,
224-
height: pageRect.height,
238+
width: textLayerRect.width,
239+
height: textLayerRect.height,
225240
},
241+
// Store coordinates are relative to text layer at current scale
242+
coordinateScale: currentScale,
243+
coordinateSystem: "textLayer", // Mark that these coordinates are relative to text layer
226244
};
245+
246+
console.log("🔍 Calculated coordinates:", coordinates);
247+
return coordinates;
227248
} catch (error) {
228249
console.error("Failed to convert selection to coordinates:", error);
229250
return null;
230251
}
231252
}
232253

233254
/**
234-
* Render highlight on the page using DOM overlay with scale-aware positioning
255+
* Render highlight on the page using text layer positioning
235256
*/
236257
private renderHighlight(highlightData: any): void {
237258
try {
@@ -245,8 +266,15 @@ export class PDFJSHighlightManager {
245266

246267
const pageElement = pageView.div;
247268

248-
// Create or get highlight layer
249-
let highlightLayer = pageElement.querySelector(
269+
// Find the text layer within the page
270+
const textLayer = pageElement.querySelector(".textLayer");
271+
if (!textLayer) {
272+
console.error("Text layer not found for rendering highlight");
273+
return;
274+
}
275+
276+
// Create or get highlight layer within the text layer
277+
let highlightLayer = textLayer.querySelector(
250278
".pdfjs-highlight-layer",
251279
);
252280
if (!highlightLayer) {
@@ -261,7 +289,7 @@ export class PDFJSHighlightManager {
261289
pointer-events: none;
262290
z-index: 1;
263291
`;
264-
pageElement.appendChild(highlightLayer);
292+
textLayer.appendChild(highlightLayer);
265293
}
266294

267295
// Create highlight element
@@ -272,40 +300,56 @@ export class PDFJSHighlightManager {
272300
highlightData.id,
273301
);
274302

275-
// Get current scale and calculate scaled coordinates
276-
const currentScale = this.pdfViewer.currentScale || 1;
277-
const creationScale = highlightData.creationScale || 1;
278303
const coords = highlightData.coordinates;
304+
const currentScale = this.pdfViewer.currentScale || 1;
279305

280-
// Scale the coordinates: (original coordinates / creation scale) * current scale
281-
const scaleRatio = currentScale / creationScale;
282-
const scaledCoords = {
283-
x: coords.x * scaleRatio,
284-
y: coords.y * scaleRatio,
285-
width: coords.width * scaleRatio,
286-
height: coords.height * scaleRatio,
306+
// For text layer coordinates, we don't need to apply additional scaling
307+
// since the text layer itself scales with PDF.js
308+
let finalCoords = {
309+
x: coords.x,
310+
y: coords.y,
311+
width: coords.width,
312+
height: coords.height,
287313
};
288314

315+
// Only apply scaling if coordinates were stored with a different coordinate system
316+
if (coords.coordinateSystem !== "textLayer") {
317+
// Legacy coordinates or coordinates from page-relative system
318+
const coordinateScale =
319+
coords.coordinateScale ||
320+
highlightData.coordinateScale ||
321+
1;
322+
const scaleRatio = currentScale / coordinateScale;
323+
finalCoords = {
324+
x: coords.x * scaleRatio,
325+
y: coords.y * scaleRatio,
326+
width: coords.width * scaleRatio,
327+
height: coords.height * scaleRatio,
328+
};
329+
}
330+
331+
console.log("🎨 Rendering highlight with coords:", finalCoords);
332+
289333
highlightElement.style.cssText = `
290334
position: absolute;
291-
left: ${scaledCoords.x}px;
292-
top: ${scaledCoords.y}px;
293-
width: ${scaledCoords.width}px;
294-
height: ${scaledCoords.height}px;
335+
left: ${finalCoords.x}px;
336+
top: ${finalCoords.y}px;
337+
width: ${finalCoords.width}px;
338+
height: ${finalCoords.height}px;
295339
background-color: ${highlightData.color};
296-
opacity: 0.3;
340+
opacity: 0.8;
297341
pointer-events: auto;
298342
border-radius: 2px;
299343
transition: opacity 0.15s ease;
300344
`;
301345

302346
// Add hover effect
303347
highlightElement.addEventListener("mouseenter", () => {
304-
highlightElement.style.opacity = "0.5";
348+
highlightElement.style.opacity = "1";
305349
});
306350

307351
highlightElement.addEventListener("mouseleave", () => {
308-
highlightElement.style.opacity = "0.3";
352+
highlightElement.style.opacity = "0.8";
309353
});
310354

311355
// Add click handler for highlight interaction
@@ -381,9 +425,13 @@ export class PDFJSHighlightManager {
381425
y: highlightData.coordinates.y,
382426
width: highlightData.coordinates.width,
383427
height: highlightData.coordinates.height,
428+
coordinateScale: highlightData.coordinates.coordinateScale,
429+
coordinateSystem:
430+
highlightData.coordinates.coordinateSystem ||
431+
"textLayer",
384432
},
385433
color: highlightData.color,
386-
opacity: 0.3,
434+
opacity: 0.8,
387435
content: highlightData.text,
388436
storage: "pdfjs" as const,
389437
createdAt: new Date().toISOString(),
@@ -404,10 +452,17 @@ export class PDFJSHighlightManager {
404452
return {
405453
id: apiAnnotation.id,
406454
color: apiAnnotation.color,
407-
coordinates: apiAnnotation.coordinates,
455+
coordinates: {
456+
...apiAnnotation.coordinates,
457+
// For legacy data without coordinateSystem, assume page-based coordinates
458+
coordinateSystem:
459+
apiAnnotation.coordinates?.coordinateSystem || "page",
460+
coordinateScale:
461+
apiAnnotation.coordinates?.coordinateScale || 1,
462+
},
408463
pageNumber: apiAnnotation.page,
409464
text: apiAnnotation.content || "",
410-
creationScale: 1, // Assume existing highlights were created at 1x scale
465+
coordinateScale: apiAnnotation.coordinates?.coordinateScale || 1,
411466
};
412467
}
413468

ts/packages/agents/browser/src/views/client/pdf/core/textSelectionManager.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,21 @@ export class TextSelectionManager {
3131
private selectionTimeout: number | null = null;
3232
private pdfViewer: any;
3333
private contextualToolbar: any = null;
34+
private ignoreSelectionChangeUntil: number = 0; // Timestamp to ignore selection changes until
3435

3536
constructor(pdfViewer: any) {
3637
this.pdfViewer = pdfViewer;
3738
this.setupEventListeners();
3839
}
3940

41+
/**
42+
* Temporarily ignore selection changes (used when highlight click opens toolbar)
43+
*/
44+
ignoreSelectionChangesFor(durationMs: number = 300): void {
45+
this.ignoreSelectionChangeUntil = Date.now() + durationMs;
46+
console.log("🔇 Ignoring selection changes for", durationMs, "ms");
47+
}
48+
4049
/**
4150
* Register a callback for selection changes
4251
*/
@@ -180,6 +189,14 @@ export class TextSelectionManager {
180189
* Process the current selection and update state
181190
*/
182191
private processSelectionChange(): void {
192+
// Check if we should ignore selection changes (e.g., after highlight click)
193+
if (Date.now() < this.ignoreSelectionChangeUntil) {
194+
console.log(
195+
"🔇 Ignoring selection change due to recent highlight click",
196+
);
197+
return;
198+
}
199+
183200
const selection = window.getSelection();
184201

185202
if (!selection || selection.rangeCount === 0) {

ts/packages/agents/browser/src/views/client/pdf/pdf-viewer.css

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,14 @@ body {
154154
height: 100%;
155155
}
156156

157-
/* PDF.js viewer container - positioned by JavaScript below toolbar */
157+
/* PDF.js viewer container - must be absolutely positioned for PDF.js */
158158
#viewerContainer {
159-
/* Position will be set dynamically by JavaScript */
159+
position: absolute;
160+
top: 0;
161+
left: 0;
162+
right: 0;
163+
bottom: 0;
164+
overflow: auto;
160165
}
161166

162167
#viewer {

ts/packages/agents/browser/src/views/client/pdf/styles/annotation-styles.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
}
3333

3434
.pdfjs-highlight:hover {
35-
opacity: 0.5 !important;
35+
opacity: 1 !important;
3636
}
3737

3838
/* Custom Highlight Annotations */

0 commit comments

Comments
 (0)