From f953fb8de03b50d4bd0b53dc2729de60679e0a4e Mon Sep 17 00:00:00 2001
From: aschmitz <29508+aschmitz@users.noreply.github.com>
Date: Thu, 10 Feb 2022 01:02:21 -0600
Subject: [PATCH] fix: don't output footnotes before their call sites
If a footnote has been output but we then decide to cancel the line it
was output on, we should cancel the output of the footnote itself.
This requires a bit of extra bookkeeping, so we know which footnotes are
relevant, but otherwise largely works using the existing
remove_placeholders code.
Includes a minor refactor of footnote area management in LayoutContext,
and a new testing utility: `tree_position`.
Closes #1564.
---
tests/layout/test_footnotes.py | 40 ++++++++++++++++++++++++++++++-
tests/testing_utils.py | 21 +++++++++++++++++
weasyprint/layout/__init__.py | 29 ++++++++++++++---------
weasyprint/layout/block.py | 43 ++++++++++++++++++++++++----------
weasyprint/layout/table.py | 6 +++--
5 files changed, 112 insertions(+), 27 deletions(-)
diff --git a/tests/layout/test_footnotes.py b/tests/layout/test_footnotes.py
index d8a9fbae2..bf8699a1a 100644
--- a/tests/layout/test_footnotes.py
+++ b/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 }', '
e
f'),
+ ('p { widows: 4 }', '
e
f'),
+ ('p + p { break-before: avoid }', 'e
f'),
+))
+def test_footnote_area_after_call(css, tail):
+ pages = render_pages('''
+
+
a
b
+ c
dx%s
''' % (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('''
diff --git a/tests/testing_utils.py b/tests/testing_utils.py
index 9b8605990..3c9722359 100644
--- a/tests/testing_utils.py
+++ b/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()
diff --git a/weasyprint/layout/__init__.py b/weasyprint/layout/__init__.py
index 93057efa4..9b9cbde50 100644
--- a/weasyprint/layout/__init__.py
+++ b/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(
diff --git a/weasyprint/layout/block.py b/weasyprint/layout/block.py
index 32e4cef5e..3bb96c86c 100644
--- a/weasyprint/layout/block.py
+++ b/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)
diff --git a/weasyprint/layout/table.py b/weasyprint/layout/table.py
index 8b2999625..30c6fc4b4 100644
--- a/weasyprint/layout/table.py
+++ b/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