From a898110e7d8d3e0f037937ce70eab13d602b2d3c Mon Sep 17 00:00:00 2001 From: Jimbo Freedman Date: Tue, 28 Jul 2026 05:36:23 +0100 Subject: [PATCH] fix: correct table cell annotation offsets under vertical padding The multi-line path in `TableCell.get_annotations` built `line_break_pos` from the post-padded `self.line_width` (which contains zero-length entries for the top and/or bottom padding added by `height.setter`) and then indexed `annotation_lines` by `no + self.vertical_padding`. Because `no` was already a padded line index, adding `vertical_padding` on top double-counted the top padding: * `valign=bottom` cells shorter than their row could write past the end of `annotation_lines`, raising `IndexError` on the second or later content line's annotations. * `valign=middle` cells silently shifted annotations onto the bottom-padding lines below the actual content, so `text[start:end]` returned whitespace or text from the next column. Fix: scan only the content-width slice of `line_width` (using a new `_content_height` recorded in `normalize_blocks`) so `no` is a content-line index 0..rows-1, and the destination `annotation_lines[no + top_pad]` then correctly references the output line holding the content. Regression tests in tests/test_table_annotation_vertical_padding.py cover the `IndexError` crash and the silent middle-/bottom-align offset shift. --- src/inscriptis/model/table.py | 23 ++- .../test_table_annotation_vertical_padding.py | 140 ++++++++++++++++++ 2 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 tests/test_table_annotation_vertical_padding.py diff --git a/src/inscriptis/model/table.py b/src/inscriptis/model/table.py index cf5a498..a16452a 100644 --- a/src/inscriptis/model/table.py +++ b/src/inscriptis/model/table.py @@ -20,6 +20,7 @@ class TableCell(Canvas): """ __slots__ = ( + "_content_height", "_width", "align", "annotation_counter", @@ -40,6 +41,7 @@ def __init__(self, align: HorizontalAlignment, valign: VerticalAlignment): self._width = None self.line_width: list[int] = [] self.vertical_padding = 0 + self._content_height = 0 def normalize_blocks(self) -> int: """Split multi-line blocks into multiple one-line blocks. @@ -52,7 +54,10 @@ def normalize_blocks(self) -> int: self.blocks = list(chain(*(line.split("\n") for line in self.blocks))) if not self.blocks: self.blocks = [""] - return len(self.blocks) + # Remember the pre-padding row count so annotation mapping can + # distinguish content lines from padding lines added by `height.setter`. + self._content_height = len(self.blocks) + return self._content_height @property def height(self) -> int: @@ -133,14 +138,26 @@ def get_annotations(self, idx: int, row_width: int) -> list[Annotation]: return result # the more challenging one - multiple cell lines - line_break_pos = list(accumulate(self.line_width)) + # + # `self.line_width` is `height`-long after vertical padding was + # applied: zero-length entries fill the top (`self.vertical_padding`) + # and bottom (for VerticalAlignment.middle) padding slots, while the + # remaining `self._content_height` entries hold the original line + # widths. Annotation `start` positions reference the *pre-padding* + # joined content (one newline between lines), so we must scan only + # the content widths to find which content line an annotation falls + # on, then offset the destination by the top padding to land on the + # correct output line. + top_pad = self.vertical_padding + content_widths = self.line_width[top_pad : top_pad + self._content_height] + line_break_pos = list(accumulate(content_widths)) annotation_lines = [[] for _ in self.blocks] # assign annotations to the corresponding line for a in self.annotations: for no, line_break in enumerate(line_break_pos): if a.start <= (line_break + no): # consider newline - annotation_lines[no + self.vertical_padding].append(a) + annotation_lines[no + top_pad].append(a) break # compute the annotation index based on its line and delta :) diff --git a/tests/test_table_annotation_vertical_padding.py b/tests/test_table_annotation_vertical_padding.py new file mode 100644 index 0000000..7af4398 --- /dev/null +++ b/tests/test_table_annotation_vertical_padding.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python + +"""Regression tests for table cell annotation offsets in the presence of +vertical padding. + +The multi-line ``TableCell.get_annotations`` path previously indexed +``annotation_lines`` by ``no + self.vertical_padding`` while iterating over +``line_break_pos`` built from the post-padded ``self.line_width``. That made +``no`` itself a padded line index, so the additional offset double-counted the +top padding and either crashed with ``IndexError`` (``valign=bottom``) or +silently shifted annotations onto padding lines (``valign=middle``). + +Each test below forces real content lines (using ``
`` rather than block +elements that emit margin-empty-lines) so the padding/content distinction is +exercised cleanly. +""" + +from inscriptis import get_annotated_text +from inscriptis.model.config import ParserConfig + + +def _annotated(html: str, rules: dict[str, list[str]]) -> dict: + return get_annotated_text(html, ParserConfig(annotation_rules=rules)) + + +def test_valign_bottom_indexerror_regression(): + """Reproduces the IndexError raised by inscriptis 2.7.3 when a cell with + ``vertical-align: bottom`` is shorter than its row and contains + annotations spanning multiple content lines. + + The short cell has 3 content lines vs. 10 in the sibling, forcing + ``vertical_padding = 7``. Pre-fix, the second ``li`` annotation matches + at padded index 7 and is then written to ``annotation_lines[7 + 7]``, + overflowing the 10-entry list. + """ + html = """ + + + + + +
+
  • a1
  • a2
  • a3
+ 		+
    +
  • b1
  • b2
  • b3
  • b4
  • b5
    • +
  • b6
  • b7
  • b8
  • b9
  • b10
    • +
+
+ """ + # Pre-fix this raises IndexError; post-fix it returns successfully and + # every left-column item is reachable via its ``li`` annotation slice. + result = _annotated(html, {"li": ["li"]}) + + li_slices = [result["text"][s:e] for s, e, t in result["label"] if t == "li"] + for item in ("a1", "a2", "a3"): + assert any(item in s for s in li_slices), f"{item!r} not covered: {li_slices}" + + +def test_valign_middle_multiline_annotation_offsets(): + """``valign=middle`` does not crash but silently places annotations on + padding lines below the content. Cell has 3 content lines centered in a + 9-line row (top_pad=3, bottom_pad=3); annotations on the 2nd and 3rd + content lines previously landed on padding lines 6 and 7 instead of + content lines 4 and 5. + """ + html = """ + + + + + +
+ x1
x2
x3 + 		L1
L2
L3
L4
L5
L6
L7
L8
L9
+ """ + result = _annotated(html, {"span": ["s"]}) + + span_slices = [result["text"][s:e] for s, e, t in result["label"] if t == "s"] + # Pre-fix: ['x1 ', '3 ', ' L7'] (2nd/3rd shifted onto padding lines). + # Post-fix: each span annotation must slice to its actual content. + assert len(span_slices) == 3, f"expected 3 span annotations, got {span_slices}" + assert "x1" in span_slices[0] + assert "x2" in span_slices[1] + assert "x3" in span_slices[2] + + +def test_valign_bottom_annotation_on_last_content_line(): + """A single annotation that starts on the last content line of a + bottom-aligned cell must point at that content. Pre-fix the loop matches + at padded index ``no = vertical_padding + rows - 1`` and the destination + ``annotation_lines[2*vertical_padding + rows - 1]`` overflows for + ``vertical_padding >= 1``. + """ + html = """ + + + + + +
+ first
second
third + 		L1
L2
L3
L4
L5
L6
+ """ + result = _annotated(html, {"b": ["b"]}) + + b = [a for a in result["label"] if a[2] == "b"] + assert len(b) == 1 + start, end, _ = b[0] + assert result["text"][start:end] == "third" + + +def test_padded_two_column_list(): + """Two columns of unequal height, so the shorter one is padded under the + default ``valign=middle``. Every list item must be reachable via its + annotation. + """ + html = """ + + + + + + + +
+
  • item1
  • item2
+ 		+
    +
  • item5
  • item6
  • item7
    • +
  • item8
  • item9
  • item10
    • +
+
+ """ + result = _annotated(html, {"li": ["li"]}) + + text = result["text"] + li_slices = [text[s:e] for s, e, t in result["label"] if t == "li"] + for item in ("item1", "item2", "item5", "item6", "item7", "item8", "item9", "item10"): + assert any(item in s for s in li_slices), f"{item!r} missing: {li_slices}"