Repository: bitcrowd/chromic_pdf
Pull request #291 (2023-12-15 13:35:20)

[#290] Fix missing footers
Fixes [[#290]](https://github.com/bitcrowd/chromic_pdf/issues/290)

Since Chrome 117 custom footers would not be displayed anymore when configured via `ChromicPDF.Template`. Reason for this was that our stylesheet was prepended to both the content markup as well as the header and footer templates. The `@page` rule contains the page margins which used to be ignored in the header and footer sections, but are not applied and lead to the footer being pushes out of its content box. Fixed by splitting `ChromicPDF.Template.styles/1` into `page_styles/1` and `header_footer_styles/1` and removing the `@page` directive from the header and footer styles.

Besides, this patch removes the `zoom` directive from the header and footer templates for Chrome >= v120, as they have removed the default 4/3 scale-up.

maltoe: > Looks good!
> 
> Just wondering about the new utils functions. Should they have their own tests? Or are they implicitely tested already?

Added some for `semver_compare/2`

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 63b1620..e0c6397 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,16 @@
 ## Unreleased
 
+### Fixed
+
+- Recover missing custom footer templates by making sure we do not add any `@page` CSS rule to the header or footer templates in `ChromicPDF.Template.source_and_options/1`. (#290)
+- Drop the default `zoom: 0.75` rule from header and footer templates for Chrome >= v120. They removed the default scale-up by 4/3, see [chromium bug #1509917](https://bugs.chromium.org/p/chromium/issues/detail?id=1509917#c3).
+
+Note that for **remote chrome** users this means they will have to explicitly specify the chrome version in the application config.
+
+```elixir
+config :chromic_pdf, chrome_version: "Google Chrome 120.0.6099.71"
+```
+
 ### Added
 
 - Add `ChromicPDF.Plug` to forward Chrome requests on an internal endpoint to a template.
@@ -7,12 +18,13 @@
 
 ### Changed
 
-- Strip styles generated in `Template.styles/1`.
+- Split `Chromic.Template.styles/1` into `page_styles/1` and `header_footer_styles/1`, and trim the stylesheets.
 - Cookies set via `:set_cookie` are now `httpOnly: true` by default.
 
 ### Removed
 
 - Dropped the outdated Phoenix example in `examples/`.
+- Deprecated `ChromicPDF.styles/1`.
 
 ## [1.14.0] - 2023-09-27
 
diff --git a/lib/chromic_pdf/pdf/chrome_runner.ex b/lib/chromic_pdf/pdf/chrome_runner.ex
index fc7544c..9eb9cfa 100644
--- a/lib/chromic_pdf/pdf/chrome_runner.ex
+++ b/lib/chromic_pdf/pdf/chrome_runner.ex
@@ -3,6 +3,8 @@
 defmodule ChromicPDF.ChromeRunner do
   @moduledoc false
 
+  import ChromicPDF.Utils, only: [system_cmd!: 3, with_app_config_cache: 2]
+
   @spec port_open(keyword()) :: port()
   def port_open(opts) do
     port_opts = append_if([:binary], :nouse_stdio, !discard_stderr?(opts))
@@ -24,17 +26,35 @@ defmodule ChromicPDF.ChromeRunner do
     {:ok, stderr}
   end
 
-  @spec version() :: [non_neg_integer()]
+  @spec version() :: binary()
   def version do
-    "#{executable()} --version"
-    |> String.to_charlist()
-    |> :os.cmd()
-    |> to_string()
-    |> String.trim()
-    |> String.split(" ")
-    |> List.last()
-    |> String.split(".")
-    |> Enum.map(&String.to_integer/1)
+    with_app_config_cache(:chrome_version, &do_version/0)
+  end
+
+  defp do_version do
+    output = system_cmd!(executable(), ["--version"], stderr_to_stdout: true)
+    [version] = Regex.run(~r/\d+\.\d+\.\d+\.\d+/, output)
+    version
+  rescue
+    e ->
+      reraise(
+        """
+        Failed to determine Chrome version.
+
+        If you're using a remote chrome instance, please configure ChromicPDF manually:
+
+            config :chromic_pdf, chrome_version: "Google Chrome 120.0.6099.71"
+
+        Afterwards, force a recompilation with:
+
+            mix deps.compile --force chromic_pdf
+
+        --- original exception --
+
+        #{Exception.format(:error, e, __STACKTRACE__)}
+        """,
+        __STACKTRACE__
+      )
   end
 
   defp shell_command(extra_args, opts) do
diff --git a/lib/chromic_pdf/pdfa/ghostscript_runner.ex b/lib/chromic_pdf/pdfa/ghostscript_runner.ex
index 06742c8..80c0283 100644
--- a/lib/chromic_pdf/pdfa/ghostscript_runner.ex
+++ b/lib/chromic_pdf/pdfa/ghostscript_runner.ex
@@ -3,7 +3,7 @@
 defmodule ChromicPDF.GhostscriptRunner do
   @moduledoc false
 
-  import ChromicPDF.Utils, only: [system_cmd!: 3]
+  import ChromicPDF.Utils, only: [semver_compare: 2, system_cmd!: 3, with_app_config_cache: 2]
 
   @default_args [
     "-sstdout=/dev/null",
@@ -104,7 +104,7 @@ defmodule ChromicPDF.GhostscriptRunner do
   end
 
   defp maybe_safer_args(command) do
-    if ghostscript_version() >= @ghostscript_safer_version do
+    if semver_compare(ghostscript_version(), @ghostscript_safer_version) in [:eq, :gt] do
       [
         "-dSAFER",
         Enum.map(command.read, &"--permit-file-read=#{&1}"),
@@ -116,7 +116,7 @@ defmodule ChromicPDF.GhostscriptRunner do
   end
 
   defp maybe_disable_new_interpreter do
-    if ghostscript_version() >= @ghostscript_new_interpreter_version do
+    if semver_compare(ghostscript_version(), @ghostscript_new_interpreter_version) in [:eq, :gt] do
       # We get segmentation faults with the new intepreter (see https://github.com/bitcrowd/chromic_pdf/issues/153):
       #
       # /usr/bin/gs exited with status 139!
@@ -138,33 +138,24 @@ defmodule ChromicPDF.GhostscriptRunner do
   end
 
   defp ghostscript_version do
-    case Application.get_env(:chromic_pdf, :ghostscript_version) do
-      nil ->
-        gsv = read_ghostscript_version()
-        Application.put_env(:chromic_pdf, :ghostscript_version, gsv)
-        gsv
-
-      gsv ->
-        gsv
-    end
+    with_app_config_cache(:ghostscript_version, &do_ghostscript_version/0)
   end
 
-  defp read_ghostscript_version do
+  defp do_ghostscript_version do
     output = system_cmd!(ghostscript_executable(), ["-v"], stderr_to_stdout: true)
-    captures = Regex.named_captures(~r/GPL Ghostscript (?<major>\d+)\.(?<minor>\d+)/, output)
-
-    case captures do
-      %{"major" => major, "minor" => minor} ->
-        [String.to_integer(major), String.to_integer(minor)]
-
-      nil ->
-        raise("""
-        Failed to determine Ghostscript version number!
-
-        Output was:
-
-        #{output}
-        """)
-    end
+    [version] = Regex.run(~r/\d+\.\d+/, output)
+    version
+  rescue
+    e ->
+      reraise(
+        """
+        Failed to determine Ghostscript version number! (#{e.__struct__})
+
+        --- original exception --
+
+        #{Exception.format(:error, e, __STACKTRACE__)}
+        """,
+        __STACKTRACE__
+      )
   end
 end
diff --git a/lib/chromic_pdf/template.ex b/lib/chromic_pdf/template.ex
index 5742e8f..785461c 100644
--- a/lib/chromic_pdf/template.ex
+++ b/lib/chromic_pdf/template.ex
@@ -16,19 +16,11 @@ defmodule ChromicPDF.Template do
   Using this module is entirely optional, but perhaps can help to avoid some common pitfalls
   arising from the slightly unintuitive and sometimes conflicting behaviour of `printToPDF`
   options and `@page` CSS styles in Chrome.
-
-  ## Page dimensions
-
-  One particularly cumbersome detail is that Chrome in headless mode does not correctly interpret
-  the `@page` CSS rule to configure the page dimensions. Resulting PDF files will always be in
-  US-letter format unless configured differently with the `paperWidth` and `paperHeight` options.
-  Experience has shown, that results will be best if the `@page` rule aligns with the values
-  passed to `printToPDF/2`, which is why this module exists to make basic page sizing easier.
-
-  To rotate a page into landscape, use the `landscape` option.
   """
 
+  import ChromicPDF.Utils, only: [semver_compare: 2]
   require EEx
+  alias ChromicPDF.ChromeRunner
 
   @type content_option :: {:content, iodata()}
 
@@ -169,14 +161,12 @@ defmodule ChromicPDF.Template do
   @spec source_and_options([content_option() | header_footer_option() | style_option()]) ::
           ChromicPDF.source_and_options()
   def source_and_options(opts) do
-    {width, height} = get_paper_size(opts)
-
+    styles = page_styles(opts)
     content = Keyword.get(opts, :content, @default_content)
-    styles = Keyword.get_lazy(opts, :styles, fn -> styles({width, height}, opts) end)
 
     %{
       source: {:html, html_concat(styles, content)},
-      opts: options(Keyword.put_new(opts, :styles, styles))
+      opts: options(opts)
     }
   end
 
@@ -221,27 +211,23 @@ defmodule ChromicPDF.Template do
   the content), including any `<style>` tags that your page needs.
   """
   @spec options() :: keyword()
-  @spec options([header_footer_option() | style_option() | {:styles, binary()}]) :: keyword()
+  @spec options([header_footer_option() | style_option()]) :: keyword()
   def options(opts \\ []) do
-    {width, height} = get_paper_size(opts)
-
     header = Keyword.get(opts, :header, "")
     footer = Keyword.get(opts, :footer, "")
-    styles = Keyword.get_lazy(opts, :styles, fn -> styles({width, height}, opts) end)
+    styles = header_footer_styles(opts)
 
     [
       print_to_pdf: %{
         preferCSSPageSize: true,
         displayHeaderFooter: true,
         headerTemplate: html_concat(styles, header),
-        footerTemplate: html_concat(styles, footer),
-        paperWidth: width,
-        paperHeight: height
+        footerTemplate: html_concat(styles, footer)
       }
     ]
   end
 
-  @styles """
+  @page_styles """
   <style>
     * {
       -webkit-print-color-adjust: <%= @webkit_print_color_adjust %>;
@@ -249,40 +235,61 @@ defmodule ChromicPDF.Template do
     }
 
     @page {
-      width: <%= @width %>;
-      height: <%= @height %>;
+      size: <%= @width %> <%= @height %>;
       margin: <%= @header_height %> 0 <%= @footer_height %>;
     }
 
+    body {
+      margin: 0;
+      padding: 0;
+    }
+  </style>
+  """
+
+  @header_footer_styles """
+  <style>
+    * {
+      -webkit-print-color-adjust: <%= @webkit_print_color_adjust %>;
+      text-rendering: <%= @text_rendering %>;
+    }
+
     #header {
       padding: 0 !important;
       height: <%= @header_height %>;
       font-size: <%= @header_font_size %>;
-      zoom: <%= @header_zoom %>;
     }
 
     #footer {
       padding: 0 !important;
       height: <%= @footer_height %>;
       font-size: <%= @footer_font_size %>;
-      zoom: <%= @footer_zoom %>;
-    }
-
-    html, body {
-      margin: 0;
-      padding: 0;
     }
   </style>
   """
 
   @doc """
-  Renders page styles for given template options.
+  Renders page styles & header/footer styles in a single template.
 
-  These base styles will configure page dimensions and header and footer heights. They also
-  remove any browser padding and margins from these elements, and set the font-size.
+  This function is deprecated. Since Chromium v117 the footer and header templates must not
+  contain any margins in a `@page` directive anymore.
 
-  Additionally, they set the zoom level of header and footer templates to 0.75 which seems to
-  make them align with the content viewport scaling better.
+  See https://github.com/bitcrowd/chromic_pdf/issues/290 for details.
+
+  Please use `page_styles/1` or `header_footer_styles/1` instead.
+  """
+  @deprecated "Use page_styles/1 or header_footer_styles/1 instead"
+  @spec styles() :: binary()
+  @spec styles([style_option()]) :: binary()
+  def styles(opts \\ []) do
+    page_styles(opts) <> header_footer_styles(opts)
+  end
+
+  @doc """
+  Renders page styles for given template options.
+
+  These base styles will configure page dimensions and apply margins for headers and footers.
+  They also remove any default browser margin from the body, and apply sane defaults for
+  rendering text in print.
 
   ## Options
 
@@ -303,32 +310,63 @@ defmodule ChromicPDF.Template do
   when explicit page dimensions are given. Hence, we provide a `landscape` option here that
   swaps the page dimensions (e.g. it turns 11.7x8.3" A4 into 8.3"x11.7").
   """
-  @spec styles() :: binary()
-  @spec styles([style_option()]) :: binary()
-  def styles(opts \\ []) do
-    styles(get_paper_size(opts), opts)
+  @spec page_styles() :: binary()
+  @spec page_styles([style_option()]) :: binary()
+  def page_styles(opts \\ []) do
+    opts
+    |> assigns_for_styles()
+    |> render_page_styles()
+    |> squish()
+  end
+
+  EEx.function_from_string(:defp, :render_page_styles, @page_styles, [:assigns])
+
+  @doc """
+  Renders header/footer styles for given template options.
+
+  These styles apply sane default to your header and footer templates. They set a default
+  fonts-size and force their height.
+
+  For Chromium before v120, they also set the zoom level of header and footer templates
+  to 0.75 which aligns them with the content viewport scaling.
+
+  https://bugs.chromium.org/p/chromium/issues/detail?id=1509917#c3
+  """
+  @spec header_footer_styles() :: binary()
+  @spec header_footer_styles([style_option()]) :: binary()
+  def header_footer_styles(opts \\ []) do
+    opts
+    |> assigns_for_styles()
+    |> render_header_footer_styles()
+    |> squish()
   end
 
-  defp styles({width, height}, opts) do
-    assigns = [
+  EEx.function_from_string(:defp, :render_header_footer_styles, @header_footer_styles, [:assigns])
+
+  defp assigns_for_styles(opts) do
+    {width, height} = get_paper_size(opts)
+
+    [
       width: "#{width}in",
       height: "#{height}in",
       header_height: Keyword.get(opts, :header_height, "0"),
       header_font_size: Keyword.get(opts, :header_font_size, "10pt"),
       footer_height: Keyword.get(opts, :footer_height, "0"),
       footer_font_size: Keyword.get(opts, :footer_font_size, "10pt"),
-      header_zoom: Keyword.get(opts, :header_zoom, "0.75"),
-      footer_zoom: Keyword.get(opts, :footer_zoom, "0.75"),
+      header_zoom: Keyword.get(opts, :header_zoom, default_zoom()),
+      footer_zoom: Keyword.get(opts, :footer_zoom, default_zoom()),
       webkit_print_color_adjust: Keyword.get(opts, :webkit_print_color_adjust, "exact"),
       text_rendering: Keyword.get(opts, :text_rendering, "auto")
     ]
-
-    assigns
-    |> render_styles()
-    |> squish()
   end
 
-  EEx.function_from_string(:defp, :render_styles, @styles, [:assigns])
+  defp default_zoom do
+    if semver_compare(ChromeRunner.version(), [120]) in [:eq, :gt] do
+      "1"
+    else
+      "0.75"
+    end
+  end
 
   # Fetches paper size from opts, translates from config or uses given {width, height} tuple.
   defp get_paper_size(manual) when tuple_size(manual) === 2, do: manual
diff --git a/lib/chromic_pdf/utils.ex b/lib/chromic_pdf/utils.ex
index 2ac0198..a3b7c66 100644
--- a/lib/chromic_pdf/utils.ex
+++ b/lib/chromic_pdf/utils.ex
@@ -119,4 +119,32 @@ defmodule ChromicPDF.Utils do
     alias Phoenix.HTML.Safe
     def rendered_to_iodata(value), do: Safe.to_iodata(value)
   end
+
+  @spec with_app_config_cache(atom, function) :: any
+  def with_app_config_cache(key, function) do
+    case Application.get_env(:chromic_pdf, key) do
+      nil ->
+        result = function.()
+        Application.put_env(:chromic_pdf, key, result)
+        result
+
+      value ->
+        value
+    end
+  end
+
+  @spec semver_compare(binary, list) :: :lt | :eq | :gt
+  def semver_compare(x, y) do
+    x
+    |> String.trim()
+    |> String.split(".")
+    |> Enum.map(&String.to_integer/1)
+    |> Enum.zip(y)
+    |> do_semver_compare()
+  end
+
+  defp do_semver_compare([]), do: :eq
+  defp do_semver_compare([{x, y} | _rest]) when x < y, do: :lt
+  defp do_semver_compare([{x, y} | _rest]) when x > y, do: :gt
+  defp do_semver_compare([{x, y} | rest]) when x == y, do: do_semver_compare(rest)
 end


Test patch:
diff --git a/test/integration/screenshot_test.exs b/test/integration/screenshot_test.exs
index 05f8a36..5e519b0 100644
--- a/test/integration/screenshot_test.exs
+++ b/test/integration/screenshot_test.exs
@@ -51,9 +51,7 @@ defmodule ChromicPDF.ScreenshotTest do
 
     @tag :identify
     test ":full_page resizes the the device dimensions to fit the content" do
-      [major | _] = version()
-
-      if major >= 91 do
+      if semver_compare(version(), [91]) in [:eq, :gt] do
         {_, _, height} = capture_screenshot_and_identify(source: {:url, "file://#{@large_html}"})
         assert height < 4000
 
diff --git a/test/unit/chromic_pdf/utils_test.exs b/test/unit/chromic_pdf/utils_test.exs
new file mode 100644
index 0000000..6302663
--- /dev/null
+++ b/test/unit/chromic_pdf/utils_test.exs
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: Apache-2.0
+
+defmodule ChromicPDF.UtilsTest do
+  use ExUnit.Case, async: true
+  import ChromicPDF.Utils
+
+  describe "semver_compare/2" do
+    test "compares a semver string with a hardcoded semver list" do
+      assert semver_compare("1.2.3", [1, 2, 3]) == :eq
+      assert semver_compare("1.2.3", [1, 2]) == :eq
+      assert semver_compare("1.1.3", [1, 2]) == :lt
+      assert semver_compare("1.3.0", [1, 2]) == :gt
+    end
+  end
+end