diff --git a/pyWebLayout/layout/page_buffer.py b/pyWebLayout/layout/page_buffer.py index 4718868..c5b69f2 100644 --- a/pyWebLayout/layout/page_buffer.py +++ b/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 - self.buffer.start_background_rendering(position, 'forward') + # Only use cache if we have the forward position mapping + # 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) diff --git a/tests/layout/test_navigation_consistency.py b/tests/layout/test_navigation_consistency.py new file mode 100644 index 0000000..b741589 --- /dev/null +++ b/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"])