Adopting @gmischler review feedbacks

py-pdf · Jul 5, 2024 · 2c37f77 · 2c37f77
1 parent a3792ea
commit 2c37f77
Show file tree

Hide file tree

Showing 14 changed files with 46 additions and 10 deletions.
diff --git a/fpdf/html.py b/fpdf/html.py
@@ -22,6 +22,10 @@
 BULLET_WIN1252 = "\x95"  # BULLET character in Windows-1252 encoding
 DEGREE_WIN1252 = "\xb0"
 HEADING_TAGS = ("h1", "h2", "h3", "h4", "h5", "h6")
+# Some of the margin values below are fractions, in order to be fully backward-compatible,
+# and due to the _scale_units() conversion performed in HTML2FPDF constructor below.
+# Those constants are formatted as Mixed Fractions, a mathematical representation
+# making clear what the closest integer value is.
 DEFAULT_TAG_STYLES = {
     # Inline tags are FontFace instances :
     "a": FontFace(color="#00f", emphasis="UNDERLINE"),
@@ -56,7 +60,7 @@
         color="#960000", b_margin=0.4, font_size_pt=8, t_margin=5 - 182 / 900
     ),
     "li": TextStyle(l_margin=5, t_margin=2),
-    "p": TextStyle(t_margin=4 + 7 / 30),
+    "p": TextStyle(),
     "pre": TextStyle(t_margin=4 + 7 / 30, font_family="Courier"),
     "ol": TextStyle(t_margin=2),
     "ul": TextStyle(t_margin=2),
@@ -367,6 +371,7 @@ def __init__(
         # nothing written yet to <pre>, remove one initial nl:
         self._pre_started = False
         self.follows_trailing_space = False  # The last write has ended with a space.
+        self.follows_heading = False  # We don't want extra space below a heading.
         self.href = ""
         self.align = ""
         self.indent = 0
@@ -493,12 +498,12 @@ def _new_paragraph(
         indent=0,
         bullet="",
     ):
-        if bullet and top_margin:
-            raise NotImplementedError(
-                f"{top_margin=} will be ignored because {bullet=} is provided, due to TextRegion._render_column_lines()"
-            )
+        # Note that currently top_margin is ignored if bullet is also provided,
+        # due to the behaviour of TextRegion._render_column_lines()
         self._end_paragraph()
         self.align = align or ""
+        if not top_margin and not self.follows_heading:
+            top_margin = self.font_size_pt / self.pdf.k
         self._paragraph = self._column.paragraph(
             text_align=align,
             line_height=line_height,
@@ -509,6 +514,7 @@ def _new_paragraph(
             bullet_string=bullet,
         )
         self.follows_trailing_space = True
+        self.follows_heading = False
 
     def _end_paragraph(self):
         self.align = ""
@@ -523,8 +529,10 @@ def _end_paragraph(self):
                 self._page_break_after_paragraph = False
 
     def _write_paragraph(self, text, link=None):
+        if not text:
+            return
         if not self._paragraph:
-            self._new_paragraph(top_margin=self.font_size_pt / self.pdf.k)
+            self._new_paragraph()
         self._paragraph.write(text, link=link)
 
     def _ln(self, h=None):
@@ -752,6 +760,10 @@ def handle_starttag(self, tag, attrs):
                 self._pre_formatted = True
                 self._pre_started = True
             if tag in BLOCK_TAGS:
+                if tag == "dd":
+                    # Not compliant with the HTML spec, but backward-compatible
+                    # cf. https://github.com/py-pdf/fpdf2/pull/1217#discussion_r1666643777
+                    self.follows_heading = True
                 self._new_paragraph(
                     align="C" if tag == "center" else None,
                     line_height=(
@@ -1006,6 +1018,7 @@ def handle_endtag(self, tag):
             )
             self.set_text_color(*font_face.color.colors255)
             self._end_paragraph()
+            self.follows_heading = True  # We don't want extra space below a heading.
         if tag in (
             "b",
             "blockquote",

diff --git a/test/html/html_features.pdf b/test/html/html_features.pdf
diff --git a/test/html/html_heading_above_below.pdf b/test/html/html_heading_above_below.pdf
diff --git a/test/html/html_heading_color_attribute.pdf b/test/html/html_heading_color_attribute.pdf
diff --git a/test/html/html_headings_line_height.pdf b/test/html/html_headings_line_height.pdf
diff --git a/test/html/html_list_vertical_margin.pdf b/test/html/html_list_vertical_margin.pdf
diff --git a/test/html/html_sections.pdf b/test/html/html_sections.pdf
diff --git a/test/html/html_superscript.pdf b/test/html/html_superscript.pdf
diff --git a/test/html/html_whitespace_handling.pdf b/test/html/html_whitespace_handling.pdf
diff --git a/test/html/test_html.py b/test/html/test_html.py
@@ -664,6 +664,29 @@ def test_html_ln_outside_p(tmp_path):
     assert_pdf_equal(pdf, HERE / "html_ln_outside_p.pdf", tmp_path)
 
 
+def test_html_sections(tmp_path):
+    pdf = FPDF()
+    pdf.add_page()
+    pdf.set_font("Helvetica", size=12)
+    pdf.write_html(
+        """
+        <section>
+           <h2>Subtitle 1</h2>
+            <section>
+              <h3>Subtitle 1.1</h3>
+              Lorem ipsum dolor sit amet, consectetur adipiscing elit,
+              sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+            </section>
+            <section>
+              <h3>Subtitle 1.2</h3>
+              Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
+            </section>
+        </section>
+        """
+    )
+    assert_pdf_equal(pdf, HERE / "html_sections.pdf", tmp_path, generate=True)
+
+
 def test_html_and_section_title_styles():  # issue 1080
     pdf = FPDF()
     pdf.add_page()

diff --git a/test/outline/html_toc.pdf b/test/outline/html_toc.pdf
diff --git a/test/outline/html_toc_2_pages.pdf b/test/outline/html_toc_2_pages.pdf
diff --git a/test/outline/html_toc_with_custom_rendering.pdf b/test/outline/html_toc_with_custom_rendering.pdf
diff --git a/test/outline/test_outline_html.py b/test/outline/test_outline_html.py
@@ -18,24 +18,24 @@ def test_html_toc(tmp_path):
         <u>Table of content:</u>
         <br>
         <toc></toc>
-        <section><h2>Subtitle 1</h2><br>
+        <section><h2>Subtitle 1</h2>
             <section><h3>Subtitle 1.1</h3>
             Lorem ipsum dolor sit amet, consectetur adipiscing elit,
             sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
             </section>
             <br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br>
-            <section><h3>Subtitle 1.2</h3><br>
+            <section><h3>Subtitle 1.2</h3>
             Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
             </section>
         </section>
         <br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br>
         <br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br>
         <section><h2>Subtitle 2</h2><br>
-            <section><h3>Subtitle 2.1</h3><br>
+            <section><h3>Subtitle 2.1</h3>
             Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
             </section>
             <br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br><br>
-            <section><h3>Subtitle 2.2</h3><br>
+            <section><h3>Subtitle 2.2</h3>
             Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
             </section>
         </section>