Fix multiline structural navigation targets

cary-rowen · cary-rowen · commit faa2c281cb10 · 2026-05-24T02:33:46.000+08:00
diff --git a/bookworm/gui/book_viewer/__init__.py b/bookworm/gui/book_viewer/__init__.py
@@ -675,7 +675,7 @@ def navigate_to_structural_element(self, element_type, forward):
                 actual_element_type,
             )
             target_position = (
-                start if not move_to_start_of_line else self.get_containing_line(stop - 1)[0]
+                start if not move_to_start_of_line else self.get_containing_line(start + 1)[0]
             )
             self.set_insertion_point(target_position)
             speech.announce(msg, True)
diff --git a/tests/test_navigation.py b/tests/test_navigation.py
@@ -0,0 +1,69 @@
+from types import SimpleNamespace
+
+import pytest
+
+from bookworm.gui import book_viewer
+from bookworm.gui.book_viewer import BookViewerWindow
+from bookworm.structured_text import SemanticElementType
+
+
+class StructuralNavigationHarness:
+    navigate_to_structural_element = BookViewerWindow.navigate_to_structural_element
+
+
+def line_bounds(text, pos):
+    pos = max(0, min(pos, len(text)))
+    start = text.rfind("\n", 0, pos) + 1
+    end = text.find("\n", pos)
+    if end == -1:
+        end = len(text)
+    return start, end
+
+
+def noop(*_args, **_kwargs):
+    pass
+
+
+@pytest.mark.parametrize(
+    "element_type",
+    [
+        SemanticElementType.LIST,
+        SemanticElementType.TABLE,
+        SemanticElementType.QUOTE,
+    ],
+)
+def test_structural_navigation_moves_multiline_elements_to_first_line(monkeypatch, element_type):
+    text = "intro\nfirst element line\nsecond element line\noutro\n"
+    start = text.index("first element line")
+    stop = text.index("\noutro")
+    viewer = StructuralNavigationHarness()
+    viewer._BookViewerWindow__latest_structured_navigation_position = None
+    viewer.insertion_point = 0
+
+    def get_semantic_element(_requested_type, _forward, _anchor):
+        return (start, stop), element_type
+
+    viewer.reader = SimpleNamespace(
+        ready=True,
+        get_semantic_element=get_semantic_element,
+    )
+    viewer.get_insertion_point = lambda: viewer.insertion_point
+    viewer.get_containing_line = lambda pos: line_bounds(text, pos)
+    viewer.get_text_by_range = lambda range_start, range_stop: text[range_start:range_stop]
+    viewer.set_insertion_point = lambda pos: setattr(viewer, "insertion_point", pos)
+
+    monkeypatch.setattr(book_viewer.speech, "announce", noop)
+    monkeypatch.setattr(
+        book_viewer.sounds,
+        "structured_navigation",
+        SimpleNamespace(play=noop),
+    )
+    monkeypatch.setattr(
+        book_viewer.reading_position_change,
+        "send",
+        noop,
+    )
+
+    BookViewerWindow.navigate_to_structural_element(viewer, element_type, True)
+
+    assert viewer.insertion_point == start

Original file line number	Diff line number	Diff line change
`@@ -675,7 +675,7 @@ def navigate_to_structural_element(self, element_type, forward):`
`675`	`675`	`actual_element_type,`
`676`	`676`	`)`
`677`	`677`	`target_position = (`
`678`		`- start if not move_to_start_of_line else self.get_containing_line(stop - 1)[0]`
	`678`	`+ start if not move_to_start_of_line else self.get_containing_line(start + 1)[0]`
`679`	`679`	`)`
`680`	`680`	`self.set_insertion_point(target_position)`
`681`	`681`	`speech.announce(msg, True)`