Fixed FPDF.local_context() style leak during page breaks - Fix #1204 (#…

…1207)

CHANGELOG.md

@@ -24,18 +24,20 @@ This can also be enabled programmatically with `warnings.simplefilter('default',
 * feature to identify the Unicode script of the input text and break it into fragments when different scripts are used, improving text shaping results
 * [`FPDF.image()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.image): now handles `keep_aspect_ratio` in combination with an enum value provided to `x`
 * file names are mentioned in errors when `fpdf2` fails to parse a SVG image
-* * feature to adjust spacing before lists via the `HTML2FPDF.list_vertical_margin` attribute
+* [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html): spacing before lists can now be adjusted via the `HTML2FPDF.list_vertical_margin` attribute
 ### Fixed
+* [`FPDF.local_context()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.local_context) used to leak styling during page breaks, when rendering `footer()` & `header()`
 * [`fpdf.drawing.DeviceCMYK`](https://py-pdf.github.io/fpdf2/fpdf/drawing.html#fpdf.drawing.DeviceCMYK) objects can now be passed to [`FPDF.set_draw_color()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.set_draw_color), [`FPDF.set_fill_color()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.set_fill_color) and [`FPDF.set_text_color()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.set_text_color) without raising a `ValueError`: [documentation](https://py-pdf.github.io/fpdf2/Text.html#text-formatting).
+* [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html): fixing rendering of `<hr>` tags, that do not trigger a page break anymore
 * individual `/Resources` directories are now properly created for each document page. This change ensures better compliance with the PDF specification but results in a slight increase in the size of PDF documents. You can still use the old behavior by setting `FPDF().single_resources_object = True`
 * line size calculation for fragments when text shaping is used
 * fixed incoherent indentation of long list entries - _cf._ [issue #1073](https://github.com/py-pdf/fpdf2/issues/1073)
 * default values for `top_margin` and `bottom_margin` in `HTML2FPDF._new_paragraph()` calls are now correctly converted into chosen document units.
+### Removed
+* an obscure and undocumented [feature](https://github.com/py-pdf/fpdf2/issues/1198) of [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html), which used to magically pass local variables as arguments.
 ### Changed
-* Removed an obscure and undocumented [feature](https://github.com/py-pdf/fpdf2/issues/1198) of [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html), which used to magically pass local variables as arguments.
 * [`FPDF.table()`](https://py-pdf.github.io/fpdf2/Tables.html) now raises an error when a single row is too high to be rendered on a single page
-* `HTML2FPDF.tag_indents` can now be non-integer. Indentation of HTML elements is now independent of font size and bullet strings.
-* No spacing controlled by `HTML2FPDF.list_vertical_margin` is created for nested HTML `<li>` elements in contrast with prior respect for `Paragraph.top_margin` when handling `Paragraph`s created when handling `<ul>` and `<ol>` start tags.
+* [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html): `tag_indents` can now be non-integer. Indentation of HTML elements is now independent of font size and bullet strings.
 
 ## [2.7.9] - 2024-05-17
 ### Added

fpdf/fpdf.py

@@ -434,7 +434,8 @@ def write_html(self, text, *args, **kwargs):
             tag_styles (dict): mapping of HTML tag names to colors
         """
         html2pdf = self.HTML2FPDF_CLASS(self, *args, **kwargs)
-        html2pdf.feed(text)
+        with self.local_context():
+            html2pdf.feed(text)
 
     def _set_min_pdf_version(self, version):
         self.pdf_version = max(self.pdf_version, version)
@@ -2746,18 +2747,7 @@ def mirror(self, origin, angle):
 
     @check_page
     @contextmanager
-    def local_context(
-        self,
-        font_family=None,
-        font_style=None,
-        font_size=None,
-        line_width=None,
-        draw_color=None,
-        fill_color=None,
-        text_color=None,
-        dash_pattern=None,
-        **kwargs,
-    ):
+    def local_context(self, **kwargs):
         """
         Creates a local graphics state, which won't affect the surrounding code.
         This method must be used as a context manager using `with`:
@@ -2770,7 +2760,11 @@ def local_context(
             allow_transparency
             auto_close
             blend_mode
+            char_vpos
+            char_spacing
             dash_pattern
+            denom_lift
+            denom_scale
             draw_color
             fill_color
             fill_opacity
@@ -2780,15 +2774,21 @@ def local_context(
             font_stretching
             intersection_rule
             line_width
+            nom_lift
+            nom_scale
             paint_rule
             stroke_cap_style
             stroke_join_style
             stroke_miter_limit
             stroke_opacity
+            sub_lift
+            sub_scale
+            sup_lift
+            sup_scale
             text_color
             text_mode
+            text_shaping
             underline
-            char_vpos
 
         Args:
             **kwargs: key-values settings to set at the beggining of this context.
@@ -2798,6 +2798,32 @@ def local_context(
                 "cannot create a local context inside an unbreakable() code block"
             )
         self._push_local_stack()
+        self._start_local_context(**kwargs)
+        yield
+        self._end_local_context()
+        self._pop_local_stack()
+
+    def _start_local_context(
+        self,
+        font_family=None,
+        font_style=None,
+        font_size=None,
+        line_width=None,
+        draw_color=None,
+        fill_color=None,
+        text_color=None,
+        dash_pattern=None,
+        **kwargs,
+    ):
+        """
+        This method starts a "q/Q" context in the page content stream,
+        and inserts operators in it to initialize all the PDF settings specified.
+        """
+        if "font_size_pt" in kwargs:
+            if font_size is not None:
+                raise ValueError("font_size & font_size_pt cannot be both provided")
+            font_size = kwargs["font_size_pt"] / self.k
+            del kwargs["font_size_pt"]
         gs = None
         for key, value in kwargs.items():
             if key in (
@@ -2815,7 +2841,23 @@ def local_context(
                 setattr(gs, key, value)
                 if key == "blend_mode":
                     self._set_min_pdf_version("1.4")
-            elif key in ("font_stretching", "text_mode", "underline", "char_vpos"):
+            elif key in (
+                "char_vpos",
+                "char_spacing",
+                "current_font",
+                "denom_lift",
+                "denom_scale",
+                "font_stretching",
+                "nom_lift",
+                "nom_scale",
+                "sub_lift",
+                "sub_scale",
+                "sup_lift",
+                "sup_scale",
+                "text_mode",
+                "text_shaping",
+                "underline",
+            ):
                 setattr(self, key, value)
             else:
                 raise ValueError(f"Unsupported setting: {key}")
@@ -2842,9 +2884,12 @@ def local_context(
             self.set_text_color(text_color)
         if dash_pattern is not None:
             self.set_dash_pattern(**dash_pattern)
-        yield
+
+    def _end_local_context(self):
+        """
+        This method ends a "q/Q" context in the page content stream.
+        """
         self._out("Q")
-        self._pop_local_stack()
 
     @property
     def accept_page_break(self):
@@ -3537,7 +3582,19 @@ def _perform_page_break_if_need_be(self, h):
 
     def _perform_page_break(self):
         x = self.x
+        # If we are in a .local_context(), we need to temporarily leave it,
+        # by popping out every GraphicsState:
+        gs_stack = []
+        while self._is_current_graphics_state_nested():
+            # This code assumes that every Graphics State in the stack
+            # has been pushed in it while adding a "q" in the PDF stream
+            # (which is what FPDF.local_context() does):
+            self._end_local_context()
+            gs_stack.append(self._pop_local_stack())
         self.add_page(same=True)
+        for prev_gs in reversed(gs_stack):
+            self._push_local_stack(prev_gs)
+            self._start_local_context(**prev_gs)
         self.x = x  # restore x but not y after drawing header
 
     def _has_next_page(self):
@@ -3813,9 +3870,7 @@ def multi_cell(
             page_break_required = self.will_page_break(h + padding.bottom)
             if page_break_required:
                 page_break_triggered = True
-                x = self.x
-                self.add_page(same=True)
-                self.x = x
+                self._perform_page_break()
                 self.y += padding.top
 
             if box_required and (text_line_index == 0 or page_break_required):
@@ -4951,7 +5006,7 @@ def insert_toc_placeholder(self, render_toc_function, pages=1):
             render_toc_function, self.page, self.y, pages
         )
         for _ in range(pages):
-            self.add_page()
+            self._perform_page_break()
 
     def set_section_title_styles(
         self,
@@ -5070,6 +5125,9 @@ def use_font_face(self, font_face: FontFace):
 
             with pdf.use_font_face(FontFace(emphasis="BOLD", color=255, size_pt=42)):
                 put_some_text()
+
+        Known limitation: in case of a page jump in this local context,
+        the temporary style may "leak" in the header() & footer().
         """
         if not font_face:
             yield

fpdf/graphics_state.py

@@ -55,7 +55,7 @@ def __init__(self, *args, **kwargs):
                 nom_lift=0.2,
                 denom_lift=0.0,
                 text_shaping=None,
-            ),
+            )
         ]
         super().__init__(*args, **kwargs)
 
@@ -76,6 +76,9 @@ def _get_current_graphics_state(self):
         gs["text_shaping"] = copy(gs["text_shaping"])
         return gs
 
+    def _is_current_graphics_state_nested(self):
+        return len(self.__statestack) > 1
+
     @property
     def draw_color(self):
         return self.__statestack[-1]["draw_color"]

fpdf/html.py

@@ -325,13 +325,10 @@ def __init__(
         self.emphasis = dict(b=False, i=False, u=False)
         self.font_size = pdf.font_size_pt
         self.set_font(pdf.font_family or "times", size=self.font_size, set_default=True)
-        self._prev_font = (pdf.font_family, self.font_size, self.emphasis)
-        self.pdf._push_local_stack()  # xpylint: disable=protected-access
 
         self._pre_formatted = False  # preserve whitespace while True.
-        self._pre_started = (
-            False  # nothing written yet to <pre>, remove one initial nl.
-        )
+        # 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 = ""
@@ -464,11 +461,7 @@ def _end_paragraph(self):
         self.align = ""
         if self._paragraph:
             self._column.end_paragraph()
-            our_context = (
-                self.pdf._pop_local_stack()  # pylint: disable=protected-access
-            )
             self._column.render()
-            self.pdf._push_local_stack(our_context)  # pylint: disable=protected-access
             self._paragraph = None
             self.follows_trailing_space = True
 
@@ -530,15 +523,12 @@ def handle_data(self, data):
         elif self._pre_formatted:  # pre blocks
             # If we want to mimick the exact HTML semantics about newlines at the
             # beginning and end of the block, then this needs some more thought.
-            s_nl = data.startswith("\n") and self._pre_started
+            if data.startswith("\n") and self._pre_started:
+                if data.endswith("\n"):
+                    data = data[1:-1]
+                else:
+                    data = data[1:]
             self._pre_started = False
-            e_nl = data.endswith("\n")
-            if s_nl and e_nl:
-                data = data[1:-1]
-            elif s_nl:
-                data = data[1:]
-            # elif e_nl:
-            #    data = data[:-1]
             self._write_data(data)
         else:
             data = _WS_SUB_PAT.sub(" ", data)
@@ -650,7 +640,13 @@ def handle_starttag(self, tag, attrs):
                 size=tag_style.size_pt or self.font_size,
             )
         if tag == "hr":
-            self.pdf.add_page(same=True)
+            self.pdf.line(
+                x1=self.pdf.l_margin,
+                y1=self.pdf.y,
+                x2=self.pdf.l_margin + self.pdf.epw,
+                y2=self.pdf.y,
+            )
+            self._write_paragraph("\n")
         if tag == "code":
             self.style_stack.append(
                 FontFace(
@@ -667,6 +663,7 @@ def handle_starttag(self, tag, attrs):
                 size=tag_style.size_pt or self.font_size,
             )
         if tag == "pre":
+            self._end_paragraph()
             self.style_stack.append(
                 FontFace(
                     family=self.font_family,
@@ -682,8 +679,8 @@ def handle_starttag(self, tag, attrs):
                 size=tag_style.size_pt or self.font_size,
             )
             self._pre_formatted = True
-            self._new_paragraph()
             self._pre_started = True
+            self._new_paragraph()
         if tag == "blockquote":
             tag_style = self.tag_styles[tag]
             if tag_style.color:
@@ -928,12 +925,12 @@ def handle_endtag(self, tag):
             self.set_font(font_face.family, font_face.size_pt)
             self.set_text_color(*font_face.color.colors255)
         if tag == "pre":
-            self._end_paragraph()
             font_face = self.style_stack.pop()
             self.set_font(font_face.family, font_face.size_pt)
             self.set_text_color(*font_face.color.colors255)
             self._pre_formatted = False
             self._pre_started = False
+            self._end_paragraph()
         if tag == "blockquote":
             self._end_paragraph()
             self.set_text_color(*self.font_color)
@@ -991,10 +988,6 @@ def feed(self, data):
         while self._tags_stack and self._tags_stack[-1] in self.HTML_UNCLOSED_TAGS:
             self._tags_stack.pop()
         self._end_paragraph()  # render the final chunk of text and clean up our local context.
-        self.pdf._pop_local_stack()  # pylint: disable=protected-access
-        if self._prev_font[0]:  # restore previously defined font settings
-            self.emphasis = self._prev_font[2]
-            self.set_font(self._prev_font[0], size=self._prev_font[1], set_default=True)
         if self._tags_stack and self.warn_on_tags_not_matching:
             LOGGER.warning("Missing HTML end tag for <%s>", self._tags_stack[-1])
 

fpdf/text_region.py

@@ -718,7 +718,7 @@ def render(self):
         _first_page_top = max(self.pdf.t_margin, self.pdf.y)
         self._render_page_lines(text_lines, _first_page_top, page_bottom)
         while text_lines:
-            self.pdf.add_page(same=True)
+            self.pdf._perform_page_break()
             self._cur_column = 0
             self._render_page_lines(text_lines, self.pdf.y, page_bottom)
 

scripts/compare-changed-pdfs.py

@@ -3,9 +3,9 @@
 # Generate a HTML page that makes it easy to visually compare all PDF files
 # that are modified in the current branch, compared to the master branch.
 
-# USAGE: ./compare-changed-pdfs.py
+# USAGE: ./compare-changed-pdfs.py [test_subdir_path]
 
-import webbrowser
+import sys, webbrowser
 from functools import partial
 from http.server import HTTPServer, SimpleHTTPRequestHandler
 from os import makedirs, scandir
@@ -30,15 +30,18 @@ def scantree_dirs(path):
             yield from scantree_dirs(entry.path)
 
 
+target_dir = sys.argv[1] if len(sys.argv) > 1 else "test"
+print(f"Processing all PDF reference files in {target_dir}")
+
 stdout = check_output("git diff --name-status master", shell=True)
 changed_pdf_files = [
     line[1:].strip()
     for line in stdout.decode("utf-8").splitlines()
-    if line.startswith("M\ttest/")
+    if line.startswith(f"M\t{target_dir}")
 ]
 
 TMP_DIR.mkdir(exist_ok=True)
-for dir in scantree_dirs(REPO_DIR / "test"):
+for dir in scantree_dirs(REPO_DIR / target_dir):
     (TMP_DIR / dir).mkdir(exist_ok=True)
 for changed_pdf_file in changed_pdf_files:
     command = f"git show master:{changed_pdf_file} > {TMP_DIR}/{changed_pdf_file}"