From c4bf165d3ea0947a8b080aa276863bb1613b5d17 Mon Sep 17 00:00:00 2001 From: Guillaume Ayoub Date: Mon, 11 Jul 2022 22:24:18 +0200 Subject: [PATCH 1/4] Follow max-height on footnote area MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This solution is by far the cleanest I’ve found to follow max-height without changing everything. Contrary to my previous tries, it keeps the original style untouched, doesn’t add dirty properties on the box and only assumes that the overflowing children are caused by the max-height set on the area. This last condition is probably true in real-life cases, but we’ll sure find soon that other users don’t have the real life we expect them to have. It also has the side effect of following the height attribute too. There’s nothing in the specification about which properties are allowed in @footnote rules, only a classical "How would one describe this in the grammar of CSS3-Page?", so … who cares? We’ll sure find soon that other users actually care more than the W3C. The existing tests pass, that’s good news, but we’ll need solid new tests. Related to #1674. --- weasyprint/layout/__init__.py | 8 +++++++- weasyprint/layout/block.py | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/weasyprint/layout/__init__.py b/weasyprint/layout/__init__.py index 501e1e1d5..792c67c77 100644 --- a/weasyprint/layout/__init__.py +++ b/weasyprint/layout/__init__.py @@ -327,7 +327,7 @@ def layout_footnote(self, footnote): """Add a footnote to the layout for this page.""" self.footnotes.remove(footnote) self.current_page_footnotes.append(footnote) - self._update_footnote_area() + return self._update_footnote_area() def unlayout_footnote(self, footnote): """Remove a footnote from the layout and return it to the waitlist.""" @@ -361,5 +361,11 @@ def _update_footnote_area(self): self.current_footnote_area.page, True, [], [], [], False, None) self.current_footnote_area.height = footnote_area.height self.page_bottom -= footnote_area.margin_height() + last_child = footnote_area.children[-1] + overflow = ( + last_child.position_y + last_child.margin_height() > + footnote_area.position_y + footnote_area.margin_height()) + return overflow else: self.current_footnote_area.height = 0 + return False diff --git a/weasyprint/layout/block.py b/weasyprint/layout/block.py index 3fcd803a3..f0a81f3ee 100644 --- a/weasyprint/layout/block.py +++ b/weasyprint/layout/block.py @@ -358,9 +358,10 @@ def _linebox_layout(context, box, index, child, new_children, page_is_empty, descendant.footnote for descendant in line.descendants() if descendant.footnote in context.footnotes) for footnote in footnotes: - context.layout_footnote(footnote) + overflow = context.layout_footnote(footnote) new_footnotes.append(footnote) overflow = ( + (overflow and not page_is_empty) or context.reported_footnotes or context.overflows_page( bottom_space, new_position_y + offset_y)) From 91a2474400103a63d931e4fcf51b3b516bb7b989 Mon Sep 17 00:00:00 2001 From: Guillaume Ayoub Date: Wed, 13 Jul 2022 10:23:57 +0200 Subject: [PATCH 2/4] Fix OTB test on Fedora The metrics and floats roundings seems to be slightly different on different platforms. Using a lower line-height value should fix the problem everywhere. Fix #1677. --- tests/draw/test_text.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/draw/test_text.py b/tests/draw/test_text.py index b008b15f3..2df85ddb5 100644 --- a/tests/draw/test_text.py +++ b/tests/draw/test_text.py @@ -845,7 +845,7 @@ def test_otb_font(assert_pixels): color: red; font-family: weasyprint-otb; font-size: 4px; - line-height: 1; + line-height: 0.8; } AaA''') From 8f5bee1d51740071ce532cfa8764b61d62261e88 Mon Sep 17 00:00:00 2001 From: Lucie Anglade Date: Wed, 13 Jul 2022 22:10:12 +0200 Subject: [PATCH 3/4] Some tests for max-height on footnote area. Related to #1674. --- tests/draw/test_footnotes.py | 199 +++++++++++++++++++++++++++++++++ tests/layout/test_footnotes.py | 54 +++++++++ 2 files changed, 253 insertions(+) diff --git a/tests/draw/test_footnotes.py b/tests/draw/test_footnotes.py index 0bea2aa7c..dae14dfac 100644 --- a/tests/draw/test_footnotes.py +++ b/tests/draw/test_footnotes.py @@ -157,3 +157,202 @@ def test_footnote_with_absolute(assert_pixels): }
ad
''') + + +@assert_no_logs +def test_footnote_max_height_1(assert_pixels): + assert_pixels(''' + RRRRRRRR_ + RRRRRRRR_ + RRRR_____ + RRRR_____ + _BBBBBB__ + _BBBBBB__ + _________ + _________ + _________ + _________ + _BBBBBB__ + _BBBBBB__ + ''', ''' + +
ab
c
d
+
ef
''') + + +@assert_no_logs +def test_footnote_max_height_2(assert_pixels): + assert_pixels(''' + RRRRRRRR_ + RRRRRRRR_ + _________ + _________ + _BBBBBB__ + _BBBBBB__ + _________ + _________ + _________ + _________ + _BBBBBB__ + _BBBBBB__ + ''', ''' + +
ab
c
d
''') + + +@assert_no_logs +def test_footnote_max_height_3(assert_pixels): + assert_pixels(''' + RRRRRRRR_ + RRRRRRRR_ + _________ + _________ + _________ + _________ + ''', ''' + +
ab
c
d
''') + + +@assert_no_logs +def test_footnote_max_height_4(assert_pixels): + assert_pixels(''' + RRRRRRRR_ + RRRRRRRR_ + RRRR_____ + RRRR_____ + _BBBBBB__ + _BBBBBB__ + RRRR_____ + RRRR_____ + _________ + _________ + _BBBBBB__ + _BBBBBB__ + ''', ''' + +
ab
c
d
+
ef
+
gh
''') + + +@assert_no_logs +def test_footnote_max_height_5(assert_pixels): + assert_pixels(''' + RRRRRRRRRR + RRRRRRRRRR + _BBBBBB___ + _BBBBBB___ + _BBBBBB___ + _BBBBBB___ + RRRR______ + RRRR______ + __________ + __________ + _BBBBBB___ + _BBBBBB___ + ''', ''' + +
ab
c
d
+
e
+
fg
''') diff --git a/tests/layout/test_footnotes.py b/tests/layout/test_footnotes.py index 8a32ea7a3..0eabf0509 100644 --- a/tests/layout/test_footnotes.py +++ b/tests/layout/test_footnotes.py @@ -643,3 +643,57 @@ def test_footnote_repagination(): assert footnote_marker.children[0].text == '1.' assert footnote_textbox.text == 'de' assert footnote_area.position_y == 5 + + +@assert_no_logs +def test_footnote_max_height(): + page1, page2 = render_pages(''' + +
ab
c
d
+
e
+
fg
''') + html1, footnote_area1= page1.children + body1, = html1.children + div, = body1.children + div_textbox, footnote_call1, footnote_call2, footnote_call3 = div.children[0].children + assert div_textbox.text == 'ab' + assert footnote_call1.children[0].text == '1' + assert footnote_call2.children[0].text == '2' + assert footnote_call3.children[0].text == '3' + footnote_line1, footnote_line2 = footnote_area1.children[0].children + footnote_marker1, footnote_content1 = footnote_line1[0].children + assert footnote_marker1.children[0].text == '1.' + assert footnote_content1.text == 'c' + footnote_marker2, footnote_content2 = footnote_line2[0].children + assert footnote_marker2.children[0].text == '2.' + assert footnote_content2.text == 'd' + + html2, footnote_area2 = page2.children + body2, = html2.children + div2, = body2.children + div_textbox2 = div2.children[0].children + assert div_textbox2.text == 'fg' + footnote_line3 = footnote_area2.children[0].children + footnote_marker3, footnote_content3 = footnote_line3[0].children + assert footnote_marker2.children[0].text == '3.' + assert footnote_content2.text == 'e' From adec43adc488b93f3947b767bf53a94c654cfe01 Mon Sep 17 00:00:00 2001 From: Guillaume Ayoub Date: Wed, 13 Jul 2022 22:45:26 +0200 Subject: [PATCH 4/4] Fix code and tests for max-height set on footnotes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug in the code was caused by the page_is_empty variable. It’s useless to test this variable here because we know that we rendered at least the current line. It’s not updated earlier, but if it were it would always be True. This commit also fixes minor bugs in the tests. Fix #1674. --- tests/draw/test_footnotes.py | 36 +++++++++++++++++++++------------- tests/layout/test_footnotes.py | 26 +++++++++++++----------- weasyprint/layout/block.py | 2 +- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/tests/draw/test_footnotes.py b/tests/draw/test_footnotes.py index dae14dfac..348c8c16e 100644 --- a/tests/draw/test_footnotes.py +++ b/tests/draw/test_footnotes.py @@ -242,6 +242,8 @@ def test_footnote_max_height_2(assert_pixels): @assert_no_logs def test_footnote_max_height_3(assert_pixels): + # This case is crazy and the rendering is not really defined, but this test + # is useful to check that there’s no endless loop. assert_pixels(''' RRRRRRRR_ RRRRRRRR_ @@ -249,6 +251,12 @@ def test_footnote_max_height_3(assert_pixels): _________ _________ _________ + _________ + _________ + _________ + _________ + _________ + _BBBBBB__ ''', '''