Skip to content

Commit 8dfbf95

Browse files
chittolinagchittolinacaio-pizzol
authored
SD-2321 - fix: negative word-spacing not preserving whitespaces (#2665)
* fix: negative word-spacing not preserving whitespaces * fix: text measurement * fix: table border being converted twice * fix: removed code added by mistake * test: add renderer and measurement tests for manual tab justify skip - renderer test: verifies word-spacing is empty for justified lines with manual tab runs but no explicit segment positions - measurement test: verifies justify still applies to normal lines without tabs (complementary sanity check) --------- Co-authored-by: Gabriel Chittolina <gabrielchittolina1@gmail.com> Co-authored-by: Caio Pizzol <caio@harbourshare.com> Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com>
1 parent 7f4133a commit 8dfbf95

4 files changed

Lines changed: 157 additions & 3 deletions

File tree

packages/layout-engine/layout-bridge/src/text-measurement.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,22 @@ const isWordChar = (char: string): boolean => {
4141
return (code >= 48 && code <= 57) || (code >= 65 && code <= 90) || (code >= 97 && code <= 122) || char === "'";
4242
};
4343

44+
const lineContainsManualTabWithoutSegments = (block: FlowBlock, line: Line): boolean => {
45+
if (block.kind !== 'paragraph') return false;
46+
if (line.segments?.some((seg) => seg.x !== undefined)) {
47+
return false;
48+
}
49+
const fromRun = line.fromRun ?? 0;
50+
const toRun = line.toRun ?? fromRun;
51+
for (let runIndex = fromRun; runIndex <= toRun; runIndex += 1) {
52+
const run = block.runs[runIndex];
53+
if (run && isTabRun(run)) {
54+
return true;
55+
}
56+
}
57+
return false;
58+
};
59+
4460
const capitalizeText = (text: string): string => {
4561
if (!text) return text;
4662
let result = '';
@@ -390,8 +406,17 @@ export function measureCharacterX(
390406
line.maxWidth ??
391407
// Fallback: if no maxWidth, approximate available width as line width (no slack)
392408
line.width;
409+
const manualTabWithoutSegments = lineContainsManualTabWithoutSegments(block, line);
393410
// Pass availableWidth to justify calculation to match painter's word-spacing
394-
const justify = getJustifyAdjustment(block, line, availableWidth, alignmentOverride);
411+
const justify = getJustifyAdjustment(
412+
block,
413+
line,
414+
availableWidth,
415+
alignmentOverride,
416+
undefined,
417+
undefined,
418+
manualTabWithoutSegments,
419+
);
395420
const alignment = alignmentOverride ?? (block.kind === 'paragraph' ? block.attrs?.alignment : undefined);
396421
// For justify alignment, the line is stretched to fill available width (slack distributed across spaces)
397422
// For center/right alignment, the line keeps its natural width and is positioned within the available space
@@ -722,7 +747,16 @@ export function findCharacterAtX(
722747
// Fallback: approximate with line width when no maxWidth is present
723748
line.width;
724749
// Pass availableWidth to justify calculation to match painter's word-spacing
725-
const justify = getJustifyAdjustment(block, line, availableWidth, alignmentOverride);
750+
const manualTabWithoutSegments = lineContainsManualTabWithoutSegments(block, line);
751+
const justify = getJustifyAdjustment(
752+
block,
753+
line,
754+
availableWidth,
755+
alignmentOverride,
756+
undefined,
757+
undefined,
758+
manualTabWithoutSegments,
759+
);
726760
const alignment = alignmentOverride ?? (block.kind === 'paragraph' ? block.attrs?.alignment : undefined);
727761
// For justify alignment, the line is stretched to fill available width (slack distributed across spaces)
728762
// For center/right alignment, the line keeps its natural width and is positioned within the available space

packages/layout-engine/layout-bridge/test/text-measurement.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,54 @@ describe('text measurement utility', () => {
570570
// Last line should NOT be justified (same width)
571571
expect(lastX).toBe(lastXNormal);
572572
});
573+
574+
it('skips justify spacing for manual tabs without explicit segments', () => {
575+
const trailingText = 'Item body';
576+
const tabWidth = 48;
577+
const block = createBlock([
578+
{ text: '1 ', fontFamily: 'Arial', fontSize: 16 },
579+
{ kind: 'tab', text: '\t', width: tabWidth },
580+
{ text: trailingText, fontFamily: 'Arial', fontSize: 16 },
581+
]);
582+
(block as any).attrs = { alignment: 'justify' };
583+
const line = baseLine({
584+
fromRun: 0,
585+
toRun: 2,
586+
toChar: trailingText.length,
587+
width: (2 + trailingText.length) * CHAR_WIDTH + tabWidth,
588+
maxWidth: 300,
589+
});
590+
591+
const targetCharOffset = 7;
592+
const baseX = measureCharacterX(block, line, targetCharOffset, line.width);
593+
const wideX = measureCharacterX(block, line, targetCharOffset, 300);
594+
expect(wideX).toBe(baseX);
595+
596+
const hitResult = findCharacterAtX(block, line, baseX, 0, 300);
597+
expect(hitResult.charOffset).toBe(targetCharOffset);
598+
});
599+
600+
it('still applies justify to lines without any tab runs', () => {
601+
// Two runs so the first line is not derived as the last line of the paragraph
602+
const block = createBlock([
603+
{ text: 'hello world foo', fontFamily: 'Arial', fontSize: 16 },
604+
{ text: 'bar baz qux', fontFamily: 'Arial', fontSize: 16 },
605+
]);
606+
(block as any).attrs = { alignment: 'justify' };
607+
const line = baseLine({
608+
fromRun: 0,
609+
toRun: 0,
610+
toChar: 15,
611+
width: 15 * CHAR_WIDTH,
612+
maxWidth: 300,
613+
});
614+
615+
const targetCharOffset = 7;
616+
const baseX = measureCharacterX(block, line, targetCharOffset, line.width);
617+
const wideX = measureCharacterX(block, line, targetCharOffset, 300);
618+
// No tabs — justify should apply, so wider available width produces different position
619+
expect(wideX).not.toBe(baseX);
620+
});
573621
});
574622

575623
describe('center and right alignment', () => {

packages/layout-engine/painters/dom/src/index.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,76 @@ describe('DomPainter', () => {
441441
expect(lines[1].style.wordSpacing).toBe('');
442442
});
443443

444+
it('skips justify for lines with manual tab runs but no explicit segment positions', () => {
445+
const tabBlock: FlowBlock = {
446+
kind: 'paragraph',
447+
id: 'tab-justify-block',
448+
runs: [
449+
{ text: '1.', fontFamily: 'Arial', fontSize: 16 },
450+
{ kind: 'tab', text: '\t', width: 48 },
451+
{ text: 'a b c d', fontFamily: 'Arial', fontSize: 16 },
452+
],
453+
attrs: { alignment: 'justify' },
454+
};
455+
456+
const tabMeasure: Measure = {
457+
kind: 'paragraph',
458+
lines: [
459+
{
460+
fromRun: 0,
461+
fromChar: 0,
462+
toRun: 2,
463+
toChar: 7,
464+
width: 60,
465+
maxWidth: 100,
466+
ascent: 12,
467+
descent: 4,
468+
lineHeight: 20,
469+
// No segments with x — this is the "manual tab without segments" case
470+
},
471+
{
472+
fromRun: 2,
473+
fromChar: 7,
474+
toRun: 2,
475+
toChar: 7,
476+
width: 0,
477+
ascent: 12,
478+
descent: 4,
479+
lineHeight: 20,
480+
},
481+
],
482+
totalHeight: 40,
483+
};
484+
485+
const tabLayout: Layout = {
486+
pageSize: { w: 200, h: 200 },
487+
pages: [
488+
{
489+
number: 1,
490+
fragments: [
491+
{
492+
kind: 'para',
493+
blockId: 'tab-justify-block',
494+
fromLine: 0,
495+
toLine: 2,
496+
x: 0,
497+
y: 0,
498+
width: 100,
499+
},
500+
],
501+
},
502+
],
503+
};
504+
505+
const painter = createTestPainter({ blocks: [tabBlock], measures: [tabMeasure] });
506+
painter.paint(tabLayout, mount);
507+
508+
const lines = Array.from(mount.querySelectorAll('.superdoc-line')) as HTMLElement[];
509+
expect(lines.length).toBeGreaterThanOrEqual(1);
510+
// Manual tab without explicit segment positions should skip justify
511+
expect(lines[0].style.wordSpacing).toBe('');
512+
});
513+
444514
it('justifies last visible line when paragraph ends with lineBreak', () => {
445515
// When a paragraph ends with <w:br/> (lineBreak), the visible text before the break
446516
// should still be justified because the "last line" is the empty line after the break.

packages/layout-engine/painters/dom/src/renderer.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5663,6 +5663,8 @@ export class DomPainter {
56635663

56645664
// Check if any segments have explicit X positioning (from tab stops)
56655665
const hasExplicitPositioning = line.segments?.some((seg) => seg.x !== undefined);
5666+
const lineContainsTabRun = runsForLine.some((run) => run.kind === 'tab');
5667+
const manualTabWithoutSegments = lineContainsTabRun && !hasExplicitPositioning;
56665668
const availableWidth = availableWidthOverride ?? line.maxWidth ?? line.width;
56675669

56685670
const justifyShouldApply = shouldApplyJustify({
@@ -5671,7 +5673,7 @@ export class DomPainter {
56715673
// Caller already folds last-line + trailing lineBreak behavior into skipJustify.
56725674
isLastLineOfParagraph: false,
56735675
paragraphEndsWithLineBreak: false,
5674-
skipJustifyOverride: skipJustify,
5676+
skipJustifyOverride: skipJustify || manualTabWithoutSegments,
56755677
});
56765678

56775679
const countSpaces = (text: string): number => {

0 commit comments

Comments
 (0)