Skip to content

fix(import): tolerate legacy text encodings, guard zero-page saves, harden editor load #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nelsonduarte merged 2 commits into from
Jul 28, 2026
Merged

fix(import): tolerate legacy text encodings, guard zero-page saves, harden editor load #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02b837b
fix(import): tolerate legacy text encodings, guard zero-page saves, h…
nelsonduarte Jul 28, 2026
b8458f0
test(import): resolve CodeQL notes on unused vars and import style
nelsonduarte Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions app/editor/canvas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,16 @@ def set_overlays(self, overlays: list):

def load(self, path: str, password: str = ""):
import fitz
if self._doc: self._doc.close()
self._doc = fitz.open(path)
if self._doc.needs_pass and password:
self._doc.authenticate(password)
if self._doc:
self._doc.close()
# Clear the reference *before* fitz.open so a corrupt PDF that
# raises never leaves ``self._doc`` pointing at an already-closed
# Document (use-after-close). Only publish on success.
self._doc = None
doc = fitz.open(path)
if doc.needs_pass and password:
doc.authenticate(password)
self._doc = doc
self._path = path
self._password = password
self._page_idx = 0
Expand Down
112 changes: 99 additions & 13 deletions app/tools/import_pdf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@
_IMG_EXTS = (".png", ".jpg", ".jpeg", ".bmp", ".tiff", ".webp", ".gif")


class _NoContent:
"""Sentinel result: a converter produced a zero-page document.

Returned (instead of the output path) when every input was empty,
rejected or unreadable so ``do_work`` never hands a page-less
``fitz.Document`` to ``doc.save()`` — which raises the cryptic
``ValueError: cannot save with zero pages``. It is truthy/non-None so
``_run_background`` treats it as success (not a cancel) and routes it
to the friendly ``tool.import.no_content`` message. ``skipped`` carries
any per-item skip count so the images path keeps its existing detail.
"""

__slots__ = ("skipped",)

def __init__(self, skipped: int = 0):
self.skipped = skipped


class TabImport(BasePage):
def __init__(self, status_fn):
super().__init__("fa5s.file-import", t("tool.import.name"),
Expand Down Expand Up @@ -155,7 +173,11 @@ def do_work(worker):
for i, src in enumerate(sources):
if worker.is_cancelled():
return None
with open(src, "r", encoding="utf-8") as f:
# errors="replace" mirrors the font pre-scan above: a
# Notepad/Excel .txt saved as Windows-1252/Latin-1 must not
# crash the real conversion with UnicodeDecodeError after
# the pre-scan told the user the file was fine.
with open(src, "r", encoding="utf-8", errors="replace") as f:
all_lines.extend(f.read().split("\n"))
all_lines.append("") # separator between files
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
Expand Down Expand Up @@ -188,6 +210,11 @@ def do_work(worker):
fontname="helv")
est_lines = max(1, len(line) * fontsize * 0.5 / max_width + 1)
y += line_height * est_lines
# Guard against an all-empty input set: doc.save() raises the
# cryptic "cannot save with zero pages" otherwise.
if doc.page_count == 0:
doc.close()
return _NoContent()
try:
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
Expand All @@ -198,7 +225,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

def _convert_images(self, sources: list, out_path: str):
# R11-L1: filter to recognised image extensions up front. The
Expand Down Expand Up @@ -239,16 +266,25 @@ def do_work(worker):
finally:
img.close()
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
# When every image was non-image/rejected/unreadable the
# doc has no pages. Signal that (carrying the skip count)
# rather than letting doc.save() raise "cannot save with
# zero pages" — which used to mask the skipped feedback.
if doc.page_count == 0:
return _NoContent(skipped)
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
finally:
doc.close()
return skipped

def on_done(skipped):
if skipped:
self._status(t("tool.import.skipped_images", n=skipped))
def on_done(result):
if isinstance(result, _NoContent):
self._on_result(result, out_path)
return
if result:
self._status(t("tool.import.skipped_images", n=result))
self._done(out_path)

self._run_background(do_work, total=max(n, 1),
Expand All @@ -264,7 +300,9 @@ def do_work(worker):
for i, src in enumerate(sources):
if worker.is_cancelled():
return None
with open(src, "r", encoding="utf-8") as f:
# errors="replace": tolerate legacy Latin-1/Windows-1252
# .md files instead of aborting with UnicodeDecodeError.
with open(src, "r", encoding="utf-8", errors="replace") as f:
all_md.append(f.read())
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
md_text = "\n\n---\n\n".join(all_md)
Expand All @@ -288,6 +326,8 @@ def do_work(worker):
page.insert_text(fitz.Point(50, y), text,
fontsize=size, fontname="helv")
y += size * 1.5
if doc.page_count == 0:
return _NoContent()
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
Expand All @@ -297,7 +337,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

def _md_to_lines(self, md: str) -> list:
"""Convert markdown to list of (text, fontsize, bold) tuples."""
Expand Down Expand Up @@ -374,6 +414,11 @@ def do_work(worker):
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
if worker.is_cancelled():
return None
# Empty/blank inputs (e.g. a text-less DOCX or an all-empty
# workbook) leave the doc page-less; signal that instead of
# letting doc.save() raise "cannot save with zero pages".
if doc.page_count == 0:
return _NoContent()
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
Expand All @@ -383,7 +428,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

# ── PPTX → PDF ──────────────────────────────────────────────────────

Expand Down Expand Up @@ -443,6 +488,11 @@ def do_work(worker):
f"{fi + 1}/{n}: {i + 1}/{total_slides}…")
if worker.is_cancelled():
return None
# Empty/blank inputs (e.g. a text-less DOCX or an all-empty
# workbook) leave the doc page-less; signal that instead of
# letting doc.save() raise "cannot save with zero pages".
if doc.page_count == 0:
return _NoContent()
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
Expand All @@ -452,7 +502,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

# ── XLSX → PDF ──────────────────────────────────────────────────────

Expand Down Expand Up @@ -487,6 +537,11 @@ def do_work(worker):
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
if worker.is_cancelled():
return None
# Empty/blank inputs (e.g. a text-less DOCX or an all-empty
# workbook) leave the doc page-less; signal that instead of
# letting doc.save() raise "cannot save with zero pages".
if doc.page_count == 0:
return _NoContent()
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
Expand All @@ -496,7 +551,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

# ── HTML → PDF ──────────────────────────────────────────────────────

Expand All @@ -516,13 +571,20 @@ def do_work(worker):
for i, src in enumerate(sources):
if worker.is_cancelled():
return None
with open(src, "r", encoding="utf-8") as f:
# errors="replace": tolerate legacy Latin-1/Windows-1252
# .html files instead of aborting with UnicodeDecodeError.
with open(src, "r", encoding="utf-8", errors="replace") as f:
soup = BeautifulSoup(f.read(), "html.parser")
lines = self._html_to_lines(soup)
self._render_lines_to_doc(doc, lines)
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
if worker.is_cancelled():
return None
# Empty/blank inputs (e.g. a text-less DOCX or an all-empty
# workbook) leave the doc page-less; signal that instead of
# letting doc.save() raise "cannot save with zero pages".
if doc.page_count == 0:
return _NoContent()
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
Expand All @@ -532,7 +594,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

def _html_to_lines(self, soup) -> list:
lines = []
Expand Down Expand Up @@ -599,6 +661,11 @@ def do_work(worker):
worker.progress.emit(i + 1, f"{i + 1}/{n}…")
if worker.is_cancelled():
return None
# Empty/blank inputs (e.g. a text-less DOCX or an all-empty
# workbook) leave the doc page-less; signal that instead of
# letting doc.save() raise "cannot save with zero pages".
if doc.page_count == 0:
return _NoContent()
# R11-M8: atomic write — avoids truncating a pre-existing
# output file if the save crashes or the process is killed.
BasePage._atomic_pdf_write(doc, out_path)
Expand All @@ -608,7 +675,7 @@ def do_work(worker):

self._run_background(do_work, total=max(n, 1),
label=t("tool.import.converting"),
on_done=lambda _r: self._done(out_path))
on_done=lambda r: self._on_result(r, out_path))

# ── Shared line renderer ────────────────────────────────────────────

Expand All @@ -633,6 +700,25 @@ def _render_lines_to_doc(self, doc, lines: list):
fontsize=size, fontname="helv")
y += size * 1.5

def _on_result(self, result, out_path: str):
"""Shared completion handler for every converter.

Surfaces a friendly, translated message when the converter yielded
no pages (``_NoContent``) instead of letting the raw
``ValueError: cannot save with zero pages`` escape; otherwise
finishes normally.
"""
if isinstance(result, _NoContent):
if result.skipped:
self._status(t("tool.import.skipped_images",
n=result.skipped))
else:
self._status(t("tool.import.no_content"))
QMessageBox.warning(self, t("msg.warning"),
t("tool.import.no_content"))
return
self._done(out_path)

def _done(self, out_path: str):
self.lbl_result.setText(f" \u2192 {os.path.basename(out_path)}")
self._status(t("tool.import.status.done", path=out_path))
Expand Down
8 changes: 8 additions & 0 deletions app/translations.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "Skipped {n} unreadable image(s)",
"tool.import.no_content": "No convertible content found — nothing to save.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -1196,6 +1197,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "Ignoradas {n} imagem(ens) ilegível(is)",
"tool.import.no_content": "Nenhum conteúdo convertível encontrado — nada para guardar.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -1807,6 +1809,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "Omitida(s) {n} imagen(es) ilegible(s)",
"tool.import.no_content": "No se encontró contenido convertible — nada que guardar.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -2418,6 +2421,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "{n} image(s) illisible(s) ignorée(s)",
"tool.import.no_content": "Aucun contenu convertible trouvé — rien à enregistrer.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -3029,6 +3033,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "{n} unlesbare(s) Bild(er) übersprungen",
"tool.import.no_content": "Kein konvertierbarer Inhalt gefunden — nichts zu speichern.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -3640,6 +3645,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "已跳过 {n} 张无法读取的图像",
"tool.import.no_content": "未找到可转换的内容 — 没有可保存的内容。",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -4251,6 +4257,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "Saltata(e) {n} immagine(i) illeggibile(i)",
"tool.import.no_content": "Nessun contenuto convertibile trovato — niente da salvare.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down Expand Up @@ -4862,6 +4869,7 @@
"tool.convert.status.epub_done": "✔ EPUB → {path}",
"tool.convert.status.starting": "→ {format} @ {dpi} DPI…",
"tool.import.skipped_images": "{n} onleesbare afbeelding(en) overgeslagen",
"tool.import.no_content": "Geen converteerbare inhoud gevonden — niets om op te slaan.",
"tool.import.status.done": "✔ PDF → {path}",
"tool.merge.status.done": "✔ PDF → {name}",
"tool.extract.status.done": "✔ {n} → {name}",
Expand Down
Loading