Skip to content

Commit

Permalink
🐛 Fix duplicated row after line break
Browse files Browse the repository at this point in the history
During the layout of a rows block, when there was not enough space left
on a page for another row, the layout of that row would return an empty
frame with a remainder that contained the entire row. When even this
empty frame (`height: 0`) did not fit onto the old page because its
margin would exceed the max height, a page break would have been
inserted _before_ that frame. However, the remainder would still be
prepended to the next page, resulting in a duplication of the row.

This commit fixes this bug by only prepending the remainder to the next
page when the frame that accompanied this remainder is actually included
in the old page, i.e. the page break is inserted _after_ that frame.
  • Loading branch information
ralfstx committed Sep 28, 2023
1 parent 6b1d7bd commit 1b62f22
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 4 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
all block types. When set to `auto`, the block will shrink to the
width of its contents.

### Fixed

* An edge case that could lead to a duplicated row after a page break
has been fixed.

## [0.5.1] - 2023-06-29

### Added
Expand Down
4 changes: 2 additions & 2 deletions src/layout-rows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function layoutRowsContent(block: RowsBlock, box: Box, doc: Document): La
doc
);

const performBreakAt = (breakIdx: number) => {
const performBreakAt = (breakIdx: number, remainder?: Block) => {
frames.splice(breakIdx);
const insertedBlock = block.insertAfterBreak?.();
remainingRows = compact([insertedBlock, remainder, ...block.rows.slice(breakIdx)]);
Expand Down Expand Up @@ -60,7 +60,7 @@ export function layoutRowsContent(block: RowsBlock, box: Box, doc: Document): La

if (remainder) {
// This row was split. Break here and include the remainder in the result.
performBreakAt(rowIdx + 1);
performBreakAt(rowIdx + 1, remainder);
break;
}

Expand Down
42 changes: 40 additions & 2 deletions test/layout-rows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Document } from '../src/document.js';
import { Frame } from '../src/layout.js';
import { layoutRowsContent } from '../src/layout-rows.js';
import { Block } from '../src/read-block.js';
import { fakeFont, range, span } from './test-utils.js';
import { extractTextRows, fakeFont, range, span } from './test-utils.js';

describe('layout-rows', () => {
let doc: Document, box: Box;
Expand Down Expand Up @@ -116,7 +116,7 @@ describe('layout-rows', () => {
);
};
const renderedIds = (frame?: Pick<Frame, 'children'>) =>
frame?.children?.map((c) => (c.objects?.[0] as any)?.name);
frame?.children?.map((c) => (c.objects?.find((o) => o.type === 'anchor') as any)?.name);
const box = { x: 20, y: 30, width: 400, height: 700 };

it('creates page break after last fitting block', () => {
Expand Down Expand Up @@ -303,6 +303,44 @@ describe('layout-rows', () => {
expect(renderedIds(frame)).toEqual(range(7).map(String));
expect(remainder).toEqual({ rows: rows.slice(7) });
});

it('includes partial remainder', () => {
const rows = makeBlocks(6);
rows.push({ id: '6', height: 70 }); // leave just enough space for a single line of text
rows.push({ id: 'extra', text: [{ text: 'line1\nline2', attrs: {} }] });

const { frame, remainder } = layoutRowsContent(
{ rows, breakInside: 'enforce-auto' as any },
box,
doc
);

expect(renderedIds(frame)).toEqual(['0', '1', '2', '3', '4', '5', '6', 'extra']);
expect(extractTextRows(frame)).toEqual(['line1']);
expect(remainder).toEqual({
rows: [expect.objectContaining({ text: [expect.objectContaining({ text: 'line2' })] })],
});
});

it('skips remainder if frame empty', () => {
const rows = makeBlocks(6);
rows.push({ id: '6', height: 90 }); // leave some space, but not enough for a line of text
rows.push({
id: 'extra',
text: [{ text: 'line1\nline2', attrs: {} }],
margin: { top: 10, left: 0, right: 0, bottom: 10 },
});

const { frame, remainder } = layoutRowsContent(
{ rows, breakInside: 'enforce-auto' as any },
box,
doc
);

expect(renderedIds(frame)).toEqual(['0', '1', '2', '3', '4', '5', '6']);
expect(extractTextRows(frame)).toEqual([]);
expect(remainder).toEqual({ rows: rows.slice(-1) });
});
});
});
});
15 changes: 15 additions & 0 deletions test/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { PDFContext, PDFDocument, PDFFont, PDFName, PDFPage, PDFRef } from 'pdf-

import { Font } from '../src/fonts.js';
import { Image } from '../src/images.js';
import { Frame } from '../src/layout.js';
import { Page } from '../src/page.js';
import { TextAttrs, TextSpan } from '../src/read-block.js';

Expand Down Expand Up @@ -86,6 +87,20 @@ export function fakePDFPage(document?: PDFDocument): PDFPage {
} as unknown as PDFPage;
}

export function extractTextRows(frame: Partial<Frame>) {
const lines = [] as string[];
frame.children?.forEach((child) => {
child.objects?.forEach((obj) => {
if (obj.type === 'text') {
obj.rows.forEach((row) => {
lines.push(row.segments.map((s) => s.text).join(', '));
});
}
});
});
return lines;
}

export function span(text: string, attrs?: TextAttrs): TextSpan {
return { text, attrs: { fontSize: 10, ...attrs } };
}
Expand Down

0 comments on commit 1b62f22

Please sign in to comment.