fix for regresssion in fw/bw navigation

pyWebLayout/layout/page_buffer.py

@@ -376,12 +376,18 @@ class BufferedPageRenderer:
         cached_page = self.buffer.get_page(position)
         if cached_page:
             # Get next position from position map
-            next_pos = self.buffer.position_map.get(position, position)
+            next_pos = self.buffer.position_map.get(position)
 
-            # Start background rendering for upcoming pages
+            # Only use cache if we have the forward position mapping
-            self.buffer.start_background_rendering(position, 'forward')
+            # Otherwise, we need to compute it
+            if next_pos is not None:
+                # Start background rendering for upcoming pages
+                self.buffer.start_background_rendering(position, 'forward')
 
-            return cached_page, next_pos
+                return cached_page, next_pos
+
+            # Cache hit for the page, but we don't have the forward position
+            # Fall through to compute it below
 
         # Render the page directly
         page, next_pos = self.layouter.render_page_forward(position, font_scale)

tests/layout/test_navigation_consistency.py

@@ -0,0 +1,359 @@
+"""
+Tests for navigation consistency in the ereader manager.
+
+This module tests that forward and backward navigation are consistent
+and produce the same content when returning to previously visited positions.
+"""
+
+import pytest
+from pyWebLayout.layout.ereader_manager import EreaderLayoutManager
+from pyWebLayout.layout.ereader_layout import RenderingPosition
+from pyWebLayout.abstract.block import Paragraph, Heading, HeadingLevel
+from pyWebLayout.abstract.inline import Word
+from pyWebLayout.style import Font
+from pyWebLayout.concrete.page import Page
+
+
+@pytest.fixture
+def sample_font():
+    """Create a standard font for testing."""
+    return Font(font_size=12, colour=(0, 0, 0))
+
+
+@pytest.fixture
+def large_document(sample_font):
+    """Create a large document with many paragraphs to ensure multiple pages."""
+    blocks = []
+
+    # Add multiple headings and paragraphs to create a substantial document
+    for section in range(5):
+        # Add a heading
+        heading = Heading(HeadingLevel.H2, sample_font)
+        heading.add_word(Word(f"Section", sample_font))
+        heading.add_word(Word(f"{section + 1}", sample_font))
+        blocks.append(heading)
+
+        # Add multiple paragraphs per section
+        for para_num in range(10):
+            p = Paragraph(sample_font)
+            # Add enough words to make substantial paragraphs
+            for word_num in range(15):
+                p.add_word(Word(f"Word_{section}_{para_num}_{word_num}", sample_font))
+            blocks.append(p)
+
+    return blocks
+
+
+def extract_text_content(page: Page) -> str:
+    """
+    Extract text content from a rendered page.
+
+    This provides a way to compare pages semantically rather than pixel-perfect.
+    Pages contain Line objects as children, and Lines contain Text objects.
+    """
+    text_content = []
+
+    # Get children from the page (these are Line objects)
+    if hasattr(page, 'children'):
+        for child in page.children:
+            # Each Line contains Text objects
+            if hasattr(child, 'text_objects') and child.text_objects:
+                for text_obj in child.text_objects:
+                    if hasattr(text_obj, '_text'):
+                        text_content.append(text_obj._text)
+                    elif hasattr(text_obj, 'text'):
+                        text_content.append(text_obj.text)
+
+    # Join all text with spaces
+    return ' '.join(text_content)
+
+
+def get_page_summary(page: Page) -> dict:
+    """
+    Get a summary of page content for comparison.
+
+    Returns a dictionary with:
+    - text_content: All text on the page
+    - child_count: Number of child objects (Lines) on the page
+    - approximate_content_size: Rough measure of content amount
+    """
+    text = extract_text_content(page)
+
+    child_count = 0
+    if hasattr(page, 'children'):
+        child_count = len(page.children)
+
+    return {
+        'text_content': text,
+        'child_count': child_count,
+        'approximate_content_size': len(text),
+        'has_content': len(text) > 0
+    }
+
+
+class TestNavigationConsistency:
+    """Tests for forward/backward navigation consistency."""
+
+    def test_forward_backward_consistency(self, large_document, tmp_path):
+        """
+        Test that navigating forward then backward returns to the same content.
+        """
+        manager = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id="test_consistency",
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Navigate forward and capture page summaries
+        forward_pages = []
+        positions = [manager.current_position.copy()]
+
+        for i in range(5):
+            page = manager.get_current_page()
+            forward_pages.append(get_page_summary(page))
+
+            next_page = manager.next_page()
+            if next_page is None:
+                break
+            positions.append(manager.current_position.copy())
+
+        # Now navigate backward
+        backward_pages = []
+        for i in range(len(positions) - 1):
+            prev_page = manager.previous_page()
+            assert prev_page is not None, f"Backward navigation {i} failed"
+
+            page = manager.get_current_page()
+            backward_pages.append(get_page_summary(page))
+
+        # Reverse backward_pages to align with forward_pages
+        backward_pages.reverse()
+
+        # Compare content (excluding the last page since we didn't go back to it)
+        for i in range(len(backward_pages)):
+            forward_summary = forward_pages[i]
+            backward_summary = backward_pages[i]
+
+            # Check that content is the same
+            assert forward_summary['text_content'] == backward_summary['text_content'], \
+                f"Page {i} content differs between forward and backward navigation"
+
+            # Check that we have content
+            assert forward_summary['has_content'], f"Page {i} has no content"
+
+    def test_forward_backward_forward_consistency(self, large_document, tmp_path):
+        """
+        Test navigating forward, backward, then forward again produces consistent content.
+        """
+        manager = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id="test_fbf_consistency",
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Navigate forward 5 pages
+        forward_first = []
+        for i in range(5):
+            page = manager.get_current_page()
+            forward_first.append(get_page_summary(page))
+            next_page = manager.next_page()
+            if next_page is None:
+                break
+
+        # Navigate backward 3 pages
+        for i in range(3):
+            prev_page = manager.previous_page()
+            assert prev_page is not None, f"Backward navigation {i} failed"
+
+        # Navigate forward 3 pages again
+        forward_second = []
+        for i in range(3):
+            page = manager.get_current_page()
+            forward_second.append(get_page_summary(page))
+            next_page = manager.next_page()
+            if next_page is None:
+                break
+
+        # Compare the content - the last 3 pages of first forward pass
+        # should match the 3 pages of second forward pass
+        for i in range(min(3, len(forward_second))):
+            original_idx = len(forward_first) - 3 + i
+            if original_idx >= 0:
+                assert forward_first[original_idx]['text_content'] == forward_second[i]['text_content'], \
+                    f"Page content differs at position {i}"
+
+    def test_position_consistency(self, large_document, tmp_path):
+        """
+        Test that positions are consistent when navigating forward and backward.
+        """
+        manager = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id="test_position_consistency",
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Navigate forward and record positions
+        forward_positions = [manager.current_position.copy()]
+        for i in range(5):
+            next_page = manager.next_page()
+            if next_page is None:
+                break
+            forward_positions.append(manager.current_position.copy())
+
+        # Navigate backward and record positions
+        backward_positions = [manager.current_position.copy()]
+        for i in range(len(forward_positions) - 1):
+            prev_page = manager.previous_page()
+            assert prev_page is not None, f"Backward navigation {i} failed"
+            backward_positions.append(manager.current_position.copy())
+
+        # Reverse backward positions to align with forward
+        backward_positions.reverse()
+
+        # Verify positions match
+        for i in range(min(len(forward_positions), len(backward_positions))):
+            assert forward_positions[i] == backward_positions[i], \
+                f"Position {i} differs: forward={forward_positions[i]}, backward={backward_positions[i]}"
+
+    def test_navigation_after_reload(self, large_document, tmp_path):
+        """
+        Test that navigation works correctly after closing and reopening the document.
+
+        This is the critical test for the bug we fixed: backward and forward navigation
+        after reloading should work without cached position mappings.
+        """
+        document_id = "test_reload_consistency"
+
+        # Session 1: Navigate forward to middle of document
+        manager1 = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id=document_id,
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Navigate forward 3 pages (not too far, so we have room to navigate)
+        for i in range(3):
+            next_page = manager1.next_page()
+            if next_page is None:
+                break
+
+        saved_position = manager1.current_position.copy()
+        manager1.shutdown()
+        del manager1
+
+        # Session 2: Reload and test navigation
+        manager2 = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id=document_id,
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Verify position was restored
+        assert manager2.current_position == saved_position, \
+            "Position was not correctly restored after reload"
+
+        # Test 1: Backward navigation works after reload (THE KEY BUG FIX)
+        current_before = manager2.current_position.copy()
+        prev_page = manager2.previous_page()
+        assert prev_page is not None, \
+            "Backward navigation after reload returned None (bug still exists!)"
+        assert manager2.current_position != current_before, \
+            "Backward navigation after reload did not change position"
+
+        # Test 2: Forward navigation works after backward
+        current_before = manager2.current_position.copy()
+        next_page = manager2.next_page()
+        assert next_page is not None, \
+            "Forward navigation after backward returned None"
+        assert manager2.current_position != current_before, \
+            "Forward navigation after backward did not change position"
+
+        # Test 3: Can go backward again
+        current_before = manager2.current_position.copy()
+        prev_page = manager2.previous_page()
+        assert prev_page is not None, \
+            "Second backward navigation returned None"
+        assert manager2.current_position != current_before, \
+            "Second backward navigation did not change position"
+
+        manager2.shutdown()
+
+    def test_multiple_navigation_cycles(self, large_document, tmp_path):
+        """
+        Test multiple cycles of forward and backward navigation.
+        """
+        manager = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id="test_cycles",
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Perform multiple cycles
+        for cycle in range(3):
+            start_position = manager.current_position.copy()
+
+            # Go forward 3 pages
+            for i in range(3):
+                next_page = manager.next_page()
+                assert next_page is not None, \
+                    f"Cycle {cycle}: Forward navigation {i} failed"
+
+            # Go backward 3 pages
+            for i in range(3):
+                prev_page = manager.previous_page()
+                assert prev_page is not None, \
+                    f"Cycle {cycle}: Backward navigation {i} failed"
+
+            # Should be back at start position
+            assert manager.current_position == start_position, \
+                f"Cycle {cycle}: Did not return to start position"
+
+    def test_content_boundaries(self, large_document, tmp_path):
+        """
+        Test navigation at document boundaries (beginning and near end).
+        """
+        manager = EreaderLayoutManager(
+            large_document,
+            page_size=(600, 800),
+            document_id="test_boundaries",
+            bookmarks_dir=str(tmp_path)
+        )
+
+        # Test at beginning
+        initial_position = manager.current_position.copy()
+        prev_page = manager.previous_page()
+        assert prev_page is None, "Should not be able to go back from beginning"
+        assert manager.current_position == initial_position, \
+            "Position should not change when at beginning"
+
+        # Navigate forward to near the end
+        page_count = 0
+        max_pages = 100  # Safety limit
+        while page_count < max_pages:
+            next_page = manager.next_page()
+            if next_page is None:
+                break
+            page_count += 1
+
+        # We should have moved from the beginning
+        assert page_count > 0, "Should have moved at least one page forward"
+
+        # Test going back from near the end
+        end_position = manager.current_position.copy()
+        prev_page = manager.previous_page()
+
+        # Should be able to go back
+        if page_count > 0:  # If we moved forward, we should be able to go back
+            assert prev_page is not None, "Should be able to go back from end"
+            assert manager.current_position != end_position, \
+                "Position should change when going back from end"
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])