Merge pull request #1566 from aschmitz/fix-1564

fix: don't output footnotes before their call sites

tests/layout/test_footnotes.py

@@ -8,7 +8,7 @@
 
 import pytest
 
-from ..testing_utils import assert_no_logs, render_pages
+from ..testing_utils import assert_no_logs, render_pages, tree_position
 
 
 @assert_no_logs
@@ -338,6 +338,44 @@ def test_reported_footnote_3():
     assert footnote3.children[0].children[1].text == '3'
 
 
+@assert_no_logs
+@pytest.mark.parametrize('css, tail', (
+    ('p { break-inside: avoid }', '<br>e<br>f'),
+    ('p { widows: 4 }', '<br>e<br>f'),
+    ('p + p { break-before: avoid }', '</p><p>e<br>f'),
+))
+def test_footnote_area_after_call(css, tail):
+    pages = render_pages('''
+        <style>
+            @font-face {src: url(weasyprint.otf); font-family: weasyprint}
+            @page {
+                size: 9px 10px;
+                background: white;
+                margin: 0;
+            }
+            body {
+                font-family: weasyprint;
+                font-size: 2px;
+                line-height: 1;
+                orphans: 2;
+                widows: 2;
+                margin: 0;
+            }
+            span {
+                float: footnote;
+            }
+            %s
+        </style>
+        <div>a<br>b</div>
+        <p>c<br>d<span>x</span>%s</p>''' % (css, tail))
+
+    footnote_call = tree_position(
+        pages, lambda box: box.element_tag == 'p::footnote-call')
+    footnote_area = tree_position(
+        pages, lambda box: type(box).__name__ == 'FootnoteAreaBox')
+    assert footnote_call < footnote_area
+
+
 @assert_no_logs
 def test_footnote_display_inline():
     page, = render_pages('''

tests/testing_utils.py

@@ -158,6 +158,27 @@ def serialize(box_list):
             for box in box_list]
 
 
+def tree_position(box_list, matcher):
+    """Return a list identifying the first matching box's tree position.
+
+    Given a list of Boxes, this function returns a list containing the first
+    (depth-first) Box that the matcher function identifies. This list can then
+    be compared to another similarly-obtained list to assert that one Box is in
+    the document tree before or after another.
+
+    box_list: a list of Box objects, possibly PageBoxes
+    matcher: a function that takes a Box and returns truthy when it matches
+
+    """
+    for i, box in enumerate(box_list):
+        if matcher(box):
+            return [i]
+        elif hasattr(box, 'children'):
+            position = tree_position(box.children, matcher)
+            if position:
+                return [i] + position
+
+
 def _parse_base(html_content, base_url=BASE_URL):
     document = FakeHTML(string=html_content, base_url=base_url)
     counter_style = CounterStyle()

weasyprint/layout/__init__.py

@@ -323,23 +323,30 @@ def get_string_or_element_for(self, store, page, name, keyword):
                 return store[name][previous_page][-1]
 
     def layout_footnote(self, footnote):
+        """Add a footnote to the layout for this page."""
         self.footnotes.remove(footnote)
         self.current_page_footnotes.append(footnote)
-        if self.current_footnote_area.height != 'auto':
-            self.page_bottom += self.current_footnote_area.margin_height()
-        self.current_footnote_area.children = self.current_page_footnotes
-        footnote_area = build.create_anonymous_boxes(
-            self.current_footnote_area.deepcopy())
-        footnote_area, _, _, _, _ = block_level_layout(
-            self, footnote_area, -inf, None, self.current_footnote_area.page,
-            True, [], [], [], False)
-        self.current_footnote_area.height = footnote_area.height
-        self.page_bottom -= footnote_area.margin_height()
+        self._update_footnote_area()
+
+    def unlayout_footnote(self, footnote):
+        """Remove a footnote from the layout and return it to the waitlist."""
+        self.footnotes.append(footnote)
+        if footnote in self.current_page_footnotes:
+            self.current_page_footnotes.remove(footnote)
+        else:
+            self.reported_footnotes.remove(footnote)
+        self._update_footnote_area()
 
     def report_footnote(self, footnote):
+        """Mark a footnote as being moved to the next page."""
         self.current_page_footnotes.remove(footnote)
         self.reported_footnotes.append(footnote)
-        self.page_bottom += self.current_footnote_area.margin_height()
+        self._update_footnote_area()
+
+    def _update_footnote_area(self):
+        """Update the page bottom size and our footnote area height."""
+        if self.current_footnote_area.height != 'auto':
+            self.page_bottom += self.current_footnote_area.margin_height()
         self.current_footnote_area.children = self.current_page_footnotes
         if self.current_footnote_area.children:
             footnote_area = build.create_anonymous_boxes(

weasyprint/layout/block.py

@@ -251,7 +251,7 @@ def _out_of_flow_layout(context, box, index, child, new_children,
             resume_at = {index: None}
             if new_children and page_break in ('avoid', 'avoid-page'):
                 result = find_earlier_page_break(
-                    new_children, absolute_boxes, fixed_boxes)
+                    context, new_children, absolute_boxes, fixed_boxes)
                 if result:
                     new_children, resume_at = result
             stop = True
@@ -294,6 +294,7 @@ def _linebox_layout(context, box, index, child, new_children, page_is_empty,
                     draw_bottom_decoration):
     abort = stop = False
     resume_at = None
+    new_footnotes = []
 
     assert len(box.children) == 1, 'line box with siblings before layout'
 
@@ -347,6 +348,7 @@ def _linebox_layout(context, box, index, child, new_children, page_is_empty,
                 if descendant.footnote in context.footnotes)
             for footnote in footnotes:
                 context.layout_footnote(footnote)
+                new_footnotes.append(footnote)
                 overflow = context.reported_footnotes or (
                     new_position_y + offset_y >
                     context.page_bottom - bottom_space)
@@ -376,7 +378,7 @@ def _linebox_layout(context, box, index, child, new_children, page_is_empty,
     if new_children:
         resume_at = {index: new_children[-1].resume_at}
 
-    return abort, stop, resume_at, position_y
+    return abort, stop, resume_at, position_y, new_footnotes
 
 
 def _in_flow_layout(context, box, index, child, new_children, page_is_empty,
@@ -479,13 +481,15 @@ def _in_flow_layout(context, box, index, child, new_children, page_is_empty,
                     not page_is_empty_with_no_children):
                 # The child content overflows the page area, display it on the
                 # next page.
-                remove_placeholders([new_child], absolute_boxes, fixed_boxes)
+                remove_placeholders(
+                    context, [new_child], absolute_boxes, fixed_boxes)
                 new_child = None
             elif (new_position_y > context.page_bottom - bottom_space and
                     not page_is_empty_with_no_children):
                 # The child border/padding overflows the page area, do the
                 # layout again with a higher bottom_space value.
-                remove_placeholders([new_child], absolute_boxes, fixed_boxes)
+                remove_placeholders(
+                    context, [new_child], absolute_boxes, fixed_boxes)
                 bottom_space += (
                     new_child.padding_bottom + new_child.border_bottom_width)
                 (new_child, resume_at, next_page, next_adjoining_margins,
@@ -513,7 +517,7 @@ def _in_flow_layout(context, box, index, child, new_children, page_is_empty,
         if page_break in ('avoid', 'avoid-page'):
             # TODO: fill the blank space at the bottom of the page
             result = find_earlier_page_break(
-                new_children, absolute_boxes, fixed_boxes)
+                context, new_children, absolute_boxes, fixed_boxes)
             if result:
                 new_children, resume_at = result
                 stop = True
@@ -537,7 +541,8 @@ def _in_flow_layout(context, box, index, child, new_children, page_is_empty,
             # This box has only rendered absolute children, keep them
             # for the next page. This is for example useful for list
             # markers.
-            remove_placeholders(new_children, absolute_boxes, fixed_boxes)
+            remove_placeholders(
+                context, new_children, absolute_boxes, fixed_boxes)
             new_children = []
 
         if new_children:
@@ -611,6 +616,7 @@ def block_container_layout(context, box, bottom_space, skip_stack,
 
     new_children = []
     next_page = {'break': 'any', 'page': None}
+    all_footnotes = []
 
     last_in_flow_child = None
 
@@ -624,6 +630,7 @@ def block_container_layout(context, box, bottom_space, skip_stack,
     for index, child in enumerate(box.children[skip:], start=(skip or 0)):
         child.position_x = position_x
         child.position_y = position_y  # doesn’t count adjoining_margins
+        new_footnotes = []
 
         if not child.is_in_normal_flow():
             abort = False
@@ -636,13 +643,15 @@ def block_container_layout(context, box, bottom_space, skip_stack,
                     (child, box, out_of_flow_resume_at))
 
         elif isinstance(child, boxes.LineBox):
-            abort, stop, resume_at, position_y = _linebox_layout(
+            (abort, stop, resume_at, position_y,
+             new_footnotes) = _linebox_layout(
                 context, box, index, child, new_children, page_is_empty,
                 absolute_boxes, fixed_boxes, adjoining_margins, bottom_space,
                 position_y, skip_stack, first_letter_style,
                 draw_bottom_decoration)
             draw_bottom_decoration |= resume_at is None
             adjoining_margins = []
+            all_footnotes += new_footnotes
 
         else:
             (abort, stop, resume_at, position_y, adjoining_margins,
@@ -656,6 +665,8 @@ def block_container_layout(context, box, bottom_space, skip_stack,
 
         if abort:
             page = child.page_values()[0]
+            for footnote in new_footnotes:
+                context.unlayout_footnote(footnote)
             return None, None, {'break': 'any', 'page': page}, [], False
         elif stop:
             break
@@ -670,6 +681,8 @@ def block_container_layout(context, box, bottom_space, skip_stack,
     if (box_is_fragmented and
             box.style['break_inside'] in ('avoid', 'avoid-page') and
             not page_is_empty):
+        for footnote in all_footnotes:
+            context.unlayout_footnote(footnote)
         return (
             None, None, {'break': 'any', 'page': None}, [], False)
 
@@ -852,7 +865,7 @@ def block_level_page_name(sibling_before, sibling_after):
         return after_page
 
 
-def find_earlier_page_break(children, absolute_boxes, fixed_boxes):
+def find_earlier_page_break(context, children, absolute_boxes, fixed_boxes):
     """Find the last possible page break in ``children``.
 
     Because of a `page-break-before: avoid` or a `page-break-after: avoid` we
@@ -874,7 +887,8 @@ def find_earlier_page_break(children, absolute_boxes, fixed_boxes):
             return None
         new_children = children[:index]
         resume_at = {0: new_children[-1].resume_at}
-        remove_placeholders(children[index:], absolute_boxes, fixed_boxes)
+        remove_placeholders(
+            context, children[index:], absolute_boxes, fixed_boxes)
         return new_children, resume_at
 
     previous_in_flow = None
@@ -904,7 +918,7 @@ def find_earlier_page_break(children, absolute_boxes, fixed_boxes):
                 boxes.BlockBox, boxes.TableBox, boxes.TableRowGroupBox)
             if isinstance(child, breakable_box_types):
                 result = find_earlier_page_break(
-                    child.children, absolute_boxes, fixed_boxes)
+                    context, child.children, absolute_boxes, fixed_boxes)
                 if result:
                     new_grand_children, resume_at = result
                     new_child = child.copy_with_children(new_grand_children)
@@ -925,7 +939,7 @@ def find_earlier_page_break(children, absolute_boxes, fixed_boxes):
         return None
 
     # TODO: don’t remove absolute and fixed placeholders found in table footers
-    remove_placeholders(children[index:], absolute_boxes, fixed_boxes)
+    remove_placeholders(context, children[index:], absolute_boxes, fixed_boxes)
     return new_children, resume_at
 
 
@@ -941,7 +955,7 @@ def reversed_enumerate(seq):
     return zip(reversed(range(len(seq))), reversed(seq))
 
 
-def remove_placeholders(box_list, absolute_boxes, fixed_boxes):
+def remove_placeholders(context, box_list, absolute_boxes, fixed_boxes):
     """Remove placeholders from absolute and fixed lists.
 
     For boxes that have been removed in find_earlier_page_break(), remove the
@@ -950,9 +964,12 @@ def remove_placeholders(box_list, absolute_boxes, fixed_boxes):
     """
     for box in box_list:
         if isinstance(box, boxes.ParentBox):
-            remove_placeholders(box.children, absolute_boxes, fixed_boxes)
+            remove_placeholders(
+                context, box.children, absolute_boxes, fixed_boxes)
         if box.style['position'] == 'absolute' and box in absolute_boxes:
             # box is not in absolute_boxes if its parent has position: relative
             absolute_boxes.remove(box)
         elif box.style['position'] == 'fixed':
             fixed_boxes.remove(box)
+        if box.footnote:
+            context.unlayout_footnote(box.footnote)

weasyprint/layout/table.py

@@ -301,7 +301,8 @@ def group_layout(group, position_y, bottom_space, page_is_empty,
                     page_break = block_level_page_break(previous_row, row)
                     if page_break == 'avoid':
                         earlier_page_break = find_earlier_page_break(
-                            new_group_children, absolute_boxes, fixed_boxes)
+                            context, new_group_children, absolute_boxes,
+                            fixed_boxes)
                         if earlier_page_break:
                             new_group_children, resume_at = earlier_page_break
                             break
@@ -390,7 +391,8 @@ def body_groups_layout(skip_stack, position_y, bottom_space,
                     page_break = block_level_page_break(previous_group, group)
                     if page_break == 'avoid':
                         earlier_page_break = find_earlier_page_break(
-                            new_table_children, absolute_boxes, fixed_boxes)
+                            context, new_table_children, absolute_boxes,
+                            fixed_boxes)
                         if earlier_page_break is not None:
                             new_table_children, resume_at = earlier_page_break
                             break