Skip to content

Commit

Permalink
fixup! Render high-res partial page views when falling back to CSS zo…
Browse files Browse the repository at this point in the history
…om (bug 1492303)

Do not render PDFPageDetailView that have been removed from their parent
PDFPageView.
  • Loading branch information
nicolo-ribaudo committed Feb 5, 2025
1 parent 7cf687a commit 7d16a5a
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 0 deletions.
75 changes: 75 additions & 0 deletions test/integration/viewer_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -948,10 +948,85 @@ describe("PDF viewer", () => {
{ pageNumber: 2, isDetailView: false, cssTransform: false },
{ pageNumber: 1, isDetailView: false, cssTransform: false },
]);

const canvasesPerPage = await page.evaluate(() =>
Array.from(
document.querySelectorAll(".canvasWrapper"),
wrapper => wrapper.childElementCount
)
);
expect(canvasesPerPage)
.withContext(`In ${browserName}, number of canvases`)
.toEqual([1, 1]);
});
});
});
});
}

describe("when immediately cancelled and re-rendered", () => {
const forEachPage = setupPages("100%", 1, {
eventBusSetup: eventBus => {
globalThis.__pageRenderedEvents = [];
eventBus.on("pagerendered", ({ pageNumber, isDetailView }) => {
globalThis.__pageRenderedEvents.push({ pageNumber, isDetailView });
});
},
});

it("propely cleans up old canvases from the dom", async () => {
await forEachPage(async (browserName, page) => {
const waitForPageRenderedEvent = filter =>
page.waitForFunction(
filterStr => {
// eslint-disable-next-line no-eval
if (globalThis.__pageRenderedEvents.some(eval(filterStr))) {
globalThis.__pageRenderedEvents = [];
return true;
}
return false;
},
{ polling: 50 },
String(filter)
);
const getCanvasCount = () =>
page.evaluate(
() =>
document.querySelector("[data-page-number='1'] .canvasWrapper")
.childElementCount
);

await waitForPageRenderedEvent(e => e.pageNumber === 1);

await page.evaluate(() => {
window.PDFViewerApplication.pdfViewer.updateScale({
drawingDelay: -1,
scaleFactor: 4,
});
});
await waitForPageRenderedEvent(
e => e.pageNumber === 1 && e.isDetailView
);
expect(await getCanvasCount())
.withContext(`In ${browserName}, after the first zoom-in`)
.toBe(2);

await page.evaluate(() => {
window.PDFViewerApplication.pdfViewer.updateScale({
drawingDelay: -1,
scaleFactor: 0.75,
});
window.PDFViewerApplication.pdfViewer.updateScale({
drawingDelay: -1,
scaleFactor: 0.25,
});
});
await waitForPageRenderedEvent(e => e.pageNumber === 1);
expect(await getCanvasCount())
.withContext(`In ${browserName}, after the two zoom-out`)
.toBe(1);
});
});
});
});
});
6 changes: 6 additions & 0 deletions web/pdf_page_detail_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,12 @@ class PDFPageDetailView extends BasePDFPageView {
}

async draw() {
// The PDFPageView might have already dropped this PDFPageDetailView. In
// that case, simply do nothing.
if (this.pageView.detailView !== this) {
return;
}

// If there is already the lower resolution canvas behind,
// we don't show the new one until when it's fully ready.
const hideUntilComplete =
Expand Down
1 change: 1 addition & 0 deletions web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
watchScroll,
} from "./ui_utils.js";
import { GenericL10n } from "web-null_l10n";
import { PDFPageDetailView } from "./pdf_page_detail_view.js";
import { PDFPageView } from "./pdf_page_view.js";
import { PDFRenderingQueue } from "./pdf_rendering_queue.js";
import { SimpleLinkService } from "./pdf_link_service.js";
Expand Down

0 comments on commit 7d16a5a

Please sign in to comment.