Skip to content

Commit 0b27ebe

Browse files
authored
Merge pull request #114 from thawn/copilot/replace-wkhtml2pdf-with-chromium
Replace wkhtmltopdf with Chromium headless for PDF generation
2 parents f49f3f4 + 42420e3 commit 0b27ebe

File tree

10 files changed

+132
-73
lines changed

10 files changed

+132
-73
lines changed

TODO.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
* import/migrate library from one computer to another - can already be done manually, needs documentation. GUI for this will not be done.
22
* test docker image and create a pre-release v2.0.0_rc1
33
* port MacOS deployment to python (automate in CI)
4-
* PDF generation: create reproducible working PDFs irrespective of the browser that the user is using. The PDF generation needs a browser engine that runs headless and supports JavaScript. make sure the PDFs created by the browser version used work for OID printing (ideally PNG images are not changed in the PDF)
5-
* selenium + browser is a good option because it allows us to try several different browsers we can first check with a live browser if the PDFs generate working codes. also, we already use sodium for the end-to-end tests so the additional packaging overhead is minimal
6-
* if selenium and browsers fail, try [playwright] (https://www.checklyhq.com/docs/learn/playwright/generating-pdfs/)
7-
* wkhtml2pdf works for Windows but development is stale and PDFs only work in a very old version with known vulnerabilities
4+
* ~~PDF generation: create reproducible working PDFs irrespective of the browser that the user is using. The PDF generation needs a browser engine that runs headless and supports JavaScript. make sure the PDFs created by the browser version used work for OID printing (ideally PNG images are not changed in the PDF)~~ **DONE: Now using Chromium headless for PDF generation**
5+
* ~~selenium + browser is a good option because it allows us to try several different browsers we can first check with a live browser if the PDFs generate working codes. also, we already use sodium for the end-to-end tests so the additional packaging overhead is minimal~~
6+
* ~~if selenium and browsers fail, try [playwright] (https://www.checklyhq.com/docs/learn/playwright/generating-pdfs/)~~
7+
* ~~wkhtml2pdf works for Windows but development is stale and PDFs only work in a very old version with known vulnerabilities~~ **DONE: Replaced with Chromium headless**
88
* port Windows deployment to python (automate in CI)
99
* save last selected albums in the browsers local storage

build/docker/Dockerfile

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@ FROM python:3.12-slim AS base
44
# Set working directory
55
WORKDIR /app
66

7-
# Install system dependencies (wget, unzip for downloads, ffmpeg for audio conversion, git for setuptools-scm)
7+
# Install system dependencies (wget, unzip for downloads, ffmpeg for audio conversion, git for setuptools-scm, chromium for PDF generation)
88
RUN apt-get update && \
99
apt-get install -y --no-install-recommends \
1010
wget \
1111
unzip \
1212
xz-utils \
1313
ca-certificates \
1414
ffmpeg \
15-
git && \
15+
git \
16+
chromium && \
1617
rm -rf /var/lib/apt/lists/*
1718

1819
# Install tttool (from releases)

src/assets/js/print.js

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -280,24 +280,40 @@ var changeNumberOfColumns = function($id) {
280280
}
281281

282282
var savePDF = function() {
283+
// First save the configuration
284+
var configVars = getElementValues($('#config'));
283285
$.post(
284286
document.baseURI,
285-
'action=save_pdf&data=' + encodeURIComponent(JSON.stringify({content: $('#wrap-all-print').html()})),
286-
function(data,textStatus,jqXHR) {
287-
if (data.success) {
288-
setTimeout(function() { window.open('/print.pdf'); }, 10000);
289-
notify($('#pdf-save'), '', 'Creating pdf, please wait about 10 s... (you need to allow popups to see the pdf. otherwise open "http://'+window.location.host+'/print.pdf" manually', 'bg-info',
290-
10000);
287+
'action=save_config&data=' + encodeURIComponent(JSON.stringify(configVars)),
288+
function(configData, textStatus, jqXHR) {
289+
if (configData.success) {
290+
// Configuration saved, now generate PDF
291+
$.post(
292+
document.baseURI,
293+
'action=save_pdf&data=' + encodeURIComponent(JSON.stringify({content: $('#wrap-all-print').html()})),
294+
function(data,textStatus,jqXHR) {
295+
if (data.success) {
296+
setTimeout(function() { window.open('/print.pdf'); }, 10000);
297+
notify($('#pdf-save'), '', 'Creating pdf, please wait about 10 s... (you need to allow popups to see the pdf. otherwise open "http://'+window.location.host+'/print.pdf" manually', 'bg-info',
298+
10000);
299+
} else {
300+
notify($('#pdf-save'), '', jqXHR.statusText, 'bg-danger',
301+
4000);
302+
}
303+
}, 'json').fail(
304+
function() {
305+
notify($('#pdf-save'), '', 'Connection error', 'bg-danger',
306+
4000);
307+
});
291308
} else {
292-
notify($('#pdf-save'), '', jqXHR.statusText, 'bg-danger',
309+
notify($('#pdf-save'), '', 'Failed to save configuration: ' + configData.error, 'bg-danger',
293310
4000);
294311
}
295312
}, 'json').fail(
296313
function() {
297-
notify($('#pdf-save'), '', 'Connection error', 'bg-danger',
314+
notify($('#pdf-save'), '', 'Connection error while saving configuration', 'bg-danger',
298315
4000);
299316
});
300-
301317
}
302318

303319
$(function() {

src/templates/pdf.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@
1010
<link href="/assets/css/bootstrap.min.css" rel="stylesheet">
1111
<link href="/assets/css/fine-uploader-new.min.css" rel="stylesheet">
1212
<link href="/assets/css/print.css" rel="stylesheet">
13+
<style>
14+
@page {
15+
size: {{ page_size }};
16+
margin: {{ page_margin }};
17+
}
18+
</style>
1319
<script src="/assets/js/jquery-3.1.1.min.js"></script>
1420
<script src="/assets/js/jquery.matchHeight-min.js"></script>
1521
</head>

src/templates/print.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ <h4 class="panel-title">
146146
data-placement="left"
147147
title="Paper size for printing. This has only been tested with A4. In case your oid codes are scaled wrongly, try adjusting this value. Valid entries are for example: A4; A4 landscape; 21cm 27.9cm; letter; 5.5in 8.5in.">
148148
</div>
149+
<div class="form-group">
150+
<label for="print_page_margin">Page margin:</label> <input type="text"
151+
name="print_page_margin" class="form-control" data-toggle="tooltip"
152+
data-placement="left"
153+
title="Page margin for PDF generation. Default is 0.5in. Valid entries are for example: 0.5in; 1cm; 10mm; 0.25in.">
154+
</div>
149155
<div id="max-tracks" class="form-group">
150156
<label for="print_max_track_controls">Maximum number of tracks in
151157
general controls:</label> <input type="number" name="print_max_track_controls"

src/ttmp32gme/db_handler.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ def initialize(self):
291291
"""
292292
INSERT OR IGNORE INTO config VALUES('host','127.0.0.1');
293293
INSERT OR IGNORE INTO config VALUES('port','10020');
294-
INSERT OR IGNORE INTO config VALUES('version','1.0.0');
294+
INSERT OR IGNORE INTO config VALUES('version','2.0.0');
295295
INSERT OR IGNORE INTO config VALUES('open_browser','TRUE');
296296
INSERT OR IGNORE INTO config VALUES('tt_dpi','1200');
297297
INSERT OR IGNORE INTO config VALUES('tt_code-dim',NULL);
@@ -300,6 +300,7 @@ def initialize(self):
300300
INSERT OR IGNORE INTO config VALUES('audio_format','mp3');
301301
INSERT OR IGNORE INTO config VALUES('print_max_track_controls','24');
302302
INSERT OR IGNORE INTO config VALUES('print_page_size','A4');
303+
INSERT OR IGNORE INTO config VALUES('print_page_margin','0.5in');
303304
INSERT OR IGNORE INTO config VALUES('print_show_cover','TRUE');
304305
INSERT OR IGNORE INTO config VALUES('print_show_album_info','TRUE');
305306
INSERT OR IGNORE INTO config VALUES('print_show_album_controls','TRUE');
@@ -1182,6 +1183,10 @@ def update_db(self) -> bool:
11821183
"ALTER TABLE gme_library ADD COLUMN player_mode TEXT DEFAULT 'music';",
11831184
],
11841185
"1.0.0": ["UPDATE config SET value='1.0.0' WHERE param='version';"],
1186+
"2.0.0": [
1187+
"UPDATE config SET value='2.0.0' WHERE param='version';",
1188+
"INSERT OR IGNORE INTO config (param, value) VALUES ('print_page_margin', '0.5in');",
1189+
],
11851190
}
11861191
current_version = Version(self.get_config_value("version"))
11871192

src/ttmp32gme/print_handler.py

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ def create_print_layout(
238238

239239

240240
def create_pdf(port: int, library_path: Optional[Path] = None) -> Optional[Path]:
241-
"""Create PDF from print layout using wkhtmltopdf.
241+
"""Create PDF from print layout using Chromium headless.
242242
243243
Args:
244244
port: Server port number for accessing the print page via HTTP
@@ -247,48 +247,53 @@ def create_pdf(port: int, library_path: Optional[Path] = None) -> Optional[Path]
247247
Returns:
248248
Path to created PDF file, or None if PDF creation failed
249249
"""
250-
wkhtmltopdf_path = get_executable_path("wkhtmltopdf")
250+
# Try multiple possible chromium binary names
251+
chromium_names = ["chromium", "chromium-browser", "google-chrome", "chrome"]
252+
chromium_path = None
251253

252-
if not wkhtmltopdf_path:
253-
logger.error("Could not create pdf, wkhtmltopdf not found.")
254+
for name in chromium_names:
255+
chromium_path = get_executable_path(name)
256+
if chromium_path:
257+
break
258+
259+
if not chromium_path:
260+
logger.error("Could not create pdf, chromium not found.")
254261
return None
255262

256263
if library_path is None:
257264
library_path = get_default_library_path()
258265

259266
pdf_file = library_path / "print.pdf"
267+
268+
# Chromium headless PDF printing arguments
269+
# --headless: Run in headless mode
270+
# --disable-gpu: Disable GPU hardware acceleration
271+
# --no-pdf-header-footer: Disable headers and footers in PDF
272+
# --print-to-pdf=<path>: Output to PDF file at specified path
273+
# Note: Margins are controlled via CSS @page rules in pdf.html (0.5in all sides)
274+
# Chromium doesn't support command-line margin parameters like wkhtmltopdf
260275
args = [
261-
wkhtmltopdf_path,
262-
"-B",
263-
"0.5in",
264-
"-T",
265-
"0.5in",
266-
"-L",
267-
"0.5in",
268-
"-R",
269-
"0.5in",
276+
chromium_path,
277+
"--headless",
278+
"--disable-gpu",
279+
"--no-pdf-header-footer",
280+
f"--print-to-pdf={pdf_file}",
270281
f"http://localhost:{port}/pdf",
271-
str(pdf_file),
272282
]
273283

274284
logger.info(f"Creating PDF: {' '.join(args)}")
275285

276286
try:
277-
if platform.system() == "Windows":
278-
# Run in background on Windows
279-
subprocess.Popen(args)
280-
else:
281-
# Run in background on Unix-like systems
282-
subprocess.Popen(args)
283-
287+
# Run in background
288+
subprocess.Popen(args)
284289
return pdf_file
285290
except Exception as e:
286291
logger.error(f"Could not create PDF: {e}")
287292
return None
288293

289294

290295
def format_print_button() -> str:
291-
"""Format the print button HTML based on platform and wkhtmltopdf availability.
296+
"""Format the print button HTML based on platform and chromium availability.
292297
293298
Returns:
294299
HTML string for print/PDF button(s) appropriate for the current platform
@@ -300,22 +305,22 @@ def format_print_button() -> str:
300305
"Save as PDF</button>"
301306
)
302307

303-
wkhtmltopdf_path = get_executable_path("wkhtmltopdf")
304-
305-
if wkhtmltopdf_path:
306-
try:
307-
result = subprocess.run(
308-
[wkhtmltopdf_path, "-V"], capture_output=True, text=True, check=True
309-
)
310-
if "0.13." in result.stdout:
311-
return (
312-
'<button type="button" class="btn btn-info" onclick="javascript:window.print()">'
313-
"Print This Page</button> "
314-
'<button type="button" id="pdf-save" class="btn btn-primary" '
315-
'data-toggle="popover" title="Save as pdf. The PDF usually prints better than the webpage.">'
316-
"Save as PDF</button>"
317-
)
318-
except Exception:
319-
pass
308+
# Try multiple possible chromium binary names
309+
chromium_names = ["chromium", "chromium-browser", "google-chrome", "chrome"]
310+
chromium_path = None
311+
312+
for name in chromium_names:
313+
chromium_path = get_executable_path(name)
314+
if chromium_path:
315+
break
316+
317+
if chromium_path:
318+
return (
319+
'<button type="button" class="btn btn-info" onclick="javascript:window.print()">'
320+
"Print This Page</button> "
321+
'<button type="button" id="pdf-save" class="btn btn-primary" '
322+
'data-toggle="popover" title="Save as pdf. The PDF usually prints better than the webpage.">'
323+
"Save as PDF</button>"
324+
)
320325

321326
return '<button type="button" class="btn btn-info" onclick="javascript:window.print()">Print This Page</button>'

src/ttmp32gme/ttmp32gme.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,13 @@ def print_post():
466466
@app.route("/pdf")
467467
def pdf_page():
468468
"""PDF generation page."""
469-
return render_template("pdf.html", strippedTitle="PDF", content=print_content)
469+
return render_template(
470+
"pdf.html",
471+
strippedTitle="PDF",
472+
content=print_content,
473+
page_size=config.get("print_page_size", "A4"),
474+
page_margin=config.get("print_page_margin", "0.5in"),
475+
)
470476

471477

472478
@app.route("/config")

tests/unit/test_db_handler.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ def test_get_config_value(self, db):
547547
"""Test get_config_value method."""
548548
version = db.get_config_value("version")
549549
assert version is not None
550-
assert version == "1.0.0"
550+
assert version == "2.0.0"
551551

552552
def test_get_config_value_not_found(self, db):
553553
"""Test get_config_value for non-existent parameter."""

tests/unit/test_print_handler.py

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,8 @@ class TestCreatePdf:
323323
@patch("ttmp32gme.print_handler.get_executable_path")
324324
@patch("ttmp32gme.print_handler.subprocess.Popen")
325325
def test_create_pdf_success(self, mock_popen, mock_get_exec):
326-
"""Test PDF creation with wkhtmltopdf available."""
327-
mock_get_exec.return_value = "/usr/bin/wkhtmltopdf"
326+
"""Test PDF creation with chromium available."""
327+
mock_get_exec.return_value = "/usr/bin/chromium"
328328

329329
with tempfile.TemporaryDirectory() as tmpdir:
330330
library_path = Path(tmpdir)
@@ -334,10 +334,14 @@ def test_create_pdf_success(self, mock_popen, mock_get_exec):
334334
assert result is not None
335335
assert result == library_path / "print.pdf"
336336
assert mock_popen.called
337+
# Verify chromium headless arguments
338+
call_args = mock_popen.call_args[0][0]
339+
assert "--headless" in call_args
340+
assert "--no-pdf-header-footer" in call_args
337341

338342
@patch("ttmp32gme.print_handler.get_executable_path")
339-
def test_create_pdf_no_wkhtmltopdf(self, mock_get_exec):
340-
"""Test PDF creation when wkhtmltopdf not found."""
343+
def test_create_pdf_no_chromium(self, mock_get_exec):
344+
"""Test PDF creation when chromium not found."""
341345
mock_get_exec.return_value = None
342346

343347
result = create_pdf(10020)
@@ -348,13 +352,29 @@ def test_create_pdf_no_wkhtmltopdf(self, mock_get_exec):
348352
@patch("ttmp32gme.print_handler.subprocess.Popen")
349353
def test_create_pdf_exception(self, mock_popen, mock_get_exec):
350354
"""Test PDF creation when subprocess fails."""
351-
mock_get_exec.return_value = "/usr/bin/wkhtmltopdf"
355+
mock_get_exec.return_value = "/usr/bin/chromium"
352356
mock_popen.side_effect = Exception("Test error")
353357

354358
result = create_pdf(10020)
355359

356360
assert result is None
357361

362+
@patch("ttmp32gme.print_handler.get_executable_path")
363+
@patch("ttmp32gme.print_handler.subprocess.Popen")
364+
def test_create_pdf_tries_multiple_names(self, mock_popen, mock_get_exec):
365+
"""Test PDF creation tries multiple chromium binary names."""
366+
# First call returns None (chromium), second returns path (chromium-browser)
367+
mock_get_exec.side_effect = [None, "/usr/bin/chromium-browser"]
368+
369+
with tempfile.TemporaryDirectory() as tmpdir:
370+
library_path = Path(tmpdir)
371+
372+
result = create_pdf(10020, library_path)
373+
374+
assert result is not None
375+
assert result == library_path / "print.pdf"
376+
assert mock_popen.called
377+
358378

359379
class TestFormatPrintButton:
360380
"""Test format_print_button function."""
@@ -371,8 +391,8 @@ def test_format_print_button_windows(self, mock_system):
371391

372392
@patch("ttmp32gme.print_handler.platform.system")
373393
@patch("ttmp32gme.print_handler.get_executable_path")
374-
def test_format_print_button_linux_no_wkhtmltopdf(self, mock_get_exec, mock_system):
375-
"""Test print button on Linux without wkhtmltopdf."""
394+
def test_format_print_button_linux_no_chromium(self, mock_get_exec, mock_system):
395+
"""Test print button on Linux without chromium."""
376396
mock_system.return_value = "Linux"
377397
mock_get_exec.return_value = None
378398

@@ -383,16 +403,10 @@ def test_format_print_button_linux_no_wkhtmltopdf(self, mock_get_exec, mock_syst
383403

384404
@patch("ttmp32gme.print_handler.platform.system")
385405
@patch("ttmp32gme.print_handler.get_executable_path")
386-
@patch("ttmp32gme.print_handler.subprocess.run")
387-
def test_format_print_button_linux_with_old_wkhtmltopdf(
388-
self, mock_run, mock_get_exec, mock_system
389-
):
390-
"""Test print button on Linux with wkhtmltopdf 0.13."""
406+
def test_format_print_button_linux_with_chromium(self, mock_get_exec, mock_system):
407+
"""Test print button on Linux with chromium available."""
391408
mock_system.return_value = "Linux"
392-
mock_get_exec.return_value = "/usr/bin/wkhtmltopdf"
393-
mock_result = Mock()
394-
mock_result.stdout = "wkhtmltopdf 0.13.0"
395-
mock_run.return_value = mock_result
409+
mock_get_exec.return_value = "/usr/bin/chromium"
396410

397411
result = format_print_button()
398412

0 commit comments

Comments
 (0)