LibreKAT/Ocideck

Watch

Fork 1

Documentmodus: visueel opslaan kan CRLF-documenten dupliceren of beschadigen #1648

New issue

Closed

opened 2026-08-21 12:00:17 +00:00 by brenno · 1 comment

brenno commented

2026-08-21 12:00:17 +00:00

Owner

Probleem

De byte-getrouwe visuele save-patcher vergelijkt regels uit de originele bron inclusief de achterblijvende carriage return met een door Quill naar LF genormaliseerde baseline. Daardoor vindt de LCS-uitlijning geen overeenkomst en kan een kleine visuele wijziging bestaande regels behouden én gewijzigde regels toevoegen.

Reproductie

Open een Markdown-document met CRLF-regeleinden.
Wijzig één woord in de visuele stand.
Sla het document op.
Vergelijk de opgeslagen regels met het origineel.

Verwacht

Alleen de bedoelde inhoud verandert en de oorspronkelijke CRLF-regeleinden blijven behouden.

Technische aanwijzing

lib/utils/source_patcher.dart splitst original, baseline en current met split('\n'); alleen original bevat daarna nog '\r'. Zowel _alignLines als de fallback-inhoudsmapping vergelijken die regels letterlijk.

Gevonden bij audit van commit d439638c6b.

## Probleem De byte-getrouwe visuele save-patcher vergelijkt regels uit de originele bron inclusief de achterblijvende carriage return met een door Quill naar LF genormaliseerde baseline. Daardoor vindt de LCS-uitlijning geen overeenkomst en kan een kleine visuele wijziging bestaande regels behouden én gewijzigde regels toevoegen. ## Reproductie 1. Open een Markdown-document met CRLF-regeleinden. 2. Wijzig één woord in de visuele stand. 3. Sla het document op. 4. Vergelijk de opgeslagen regels met het origineel. ## Verwacht Alleen de bedoelde inhoud verandert en de oorspronkelijke CRLF-regeleinden blijven behouden. ## Technische aanwijzing lib/utils/source_patcher.dart splitst original, baseline en current met split('\n'); alleen original bevat daarna nog '\r'. Zowel _alignLines als de fallback-inhoudsmapping vergelijken die regels letterlijk. Gevonden bij audit van commit d439638c6bd1b519d68d87680cb022b7d5eddc85.

brenno added the

bug

triage

labels

2026-08-21 12:00:17 +00:00

brenno added

accepted

and removed

triage

labels

2026-08-21 22:51:52 +00:00

brenno commented

2026-08-21 22:51:53 +00:00

Author

Owner

Triage: accepted

Bevestigd tegen main (e93ef205c). lib/utils/source_patcher.dart splitst alle drie de versies met split('\n'). De baseline loopt via normalizeRichTextMarkdown, en die zet in lib/utils/markdown_paste_cleanup.dart élke \r\n en \r om naar \n. original is de enige die de \r houdt. Gevolg: in een CRLF-document is er geen enkele regel die aan a[i] == b[j] voldoet — de LCS vindt nul overeenkomsten, de inhoudsfallback in _applyEdits evenmin (die vergelijkt op dezelfde letterlijke tekst), en de patcher levert origineel plus gewijzigd naast elkaar op.

Oplossingsrichting

Splits original in paren (regeltekst, regeleinde) en vergelijk uitsluitend op de tekst; het regeleinde reist mee als eigenschap van de regel. Nieuwe regels uit current krijgen het dominante regeleinde van het document (eerste voorkomen van \r\n bepaalt het) — zo wordt een CRLF-document niet stilletjes gemengd. De publieke handtekening van patchVisualEdits hoeft niet te wijzigen; het is één laag onder de LCS.

Bewust niet: original óók normaliseren vóór het patchen. Dat maakt de functie byte-ontrouw voor precies het geval waarvoor ze bestaat.

Regressietest (verplicht)

test/source_patcher_test.dart: CRLF-bron, één woord gewijzigd → precies die ene regel verschilt, alle regeleinden nog \r\n.
test/visual_save_byte_faithful_test.dart: hetzelfde langs de echte round-trip, zodat de baseline-normalisatie in de test zit en niet in de aanname.

Beide eerst rood.

Kosten

Eén bestand, plus twee tests. Geen l10n, geen formaatwijziging, geen andere aanroeper (patchVisualEdits wordt alleen door document_save_actions.dart gebruikt).

Prioriteit

Hoog binnen deze reeks: het is stille schade aan het bestand van de gebruiker, in de stand die standaard aan staat.

## Triage: accepted **Bevestigd tegen `main` (e93ef205c).** `lib/utils/source_patcher.dart` splitst alle drie de versies met `split('\n')`. De baseline loopt via `normalizeRichTextMarkdown`, en die zet in `lib/utils/markdown_paste_cleanup.dart` élke `\r\n` en `\r` om naar `\n`. `original` is de enige die de `\r` houdt. Gevolg: in een CRLF-document is er geen enkele regel die aan `a[i] == b[j]` voldoet — de LCS vindt nul overeenkomsten, de inhoudsfallback in `_applyEdits` evenmin (die vergelijkt op dezelfde letterlijke tekst), en de patcher levert origineel plus gewijzigd naast elkaar op. ## Oplossingsrichting Splits `original` in paren (regeltekst, regeleinde) en vergelijk uitsluitend op de tekst; het regeleinde reist mee als eigenschap van de regel. Nieuwe regels uit `current` krijgen het dominante regeleinde van het document (eerste voorkomen van `\r\n` bepaalt het) — zo wordt een CRLF-document niet stilletjes gemengd. De publieke handtekening van `patchVisualEdits` hoeft niet te wijzigen; het is één laag onder de LCS. Bewust niet: `original` óók normaliseren vóór het patchen. Dat maakt de functie byte-ontrouw voor precies het geval waarvoor ze bestaat. ## Regressietest (verplicht) - `test/source_patcher_test.dart`: CRLF-bron, één woord gewijzigd → precies die ene regel verschilt, alle regeleinden nog `\r\n`. - `test/visual_save_byte_faithful_test.dart`: hetzelfde langs de echte round-trip, zodat de baseline-normalisatie in de test zit en niet in de aanname. Beide eerst rood. ## Kosten Eén bestand, plus twee tests. Geen l10n, geen formaatwijziging, geen andere aanroeper (`patchVisualEdits` wordt alleen door `document_save_actions.dart` gebruikt). ## Prioriteit Hoog binnen deze reeks: het is stille schade aan het bestand van de gebruiker, in de stand die standaard aan staat.

brenno added the

in-progress

label

2026-08-21 23:36:54 +00:00

brenno referenced this issue from a commit

2026-08-22 00:18:57 +00:00

fix: eerste batch accepted issues (#1700, #1659, #1675, #1679, #1658, #1657, #1665, #1667, #1649, #1648, #1651, #1685)

brenno referenced this issue

2026-08-22 00:19:14 +00:00

fix: alle accepted issues in één batch (17 issues) #1706

brenno referenced this issue from a commit

2026-08-22 00:31:53 +00:00