Skip to content

Writer fidelity: run text, escaping, reproducible bytes, paragraph and table properties - #7

Merged
dskripchenko merged 10 commits into
dskripchenko:mainfrom
One-Learning-Community:feat/writer-fidelity
Oct 27, 2026
Merged

dskripchenko merged 10 commits into
dskripchenko:mainfrom
One-Learning-Community:feat/writer-fidelity

Conversation

@serpentblade

Copy link
Copy Markdown
Contributor

Thanks for merging #3 and #4. This PR collects the rest of the gaps we found while building a DOCX exporter on the library. It is one PR instead of four. The commits are grouped by area, and each one passes the full suite and composer stan on its own, so any group can be dropped or cherry-picked.

Area Commits
Run text fix: write tabs and newlines in run text as w:tab and w:br
fix: XmlEscape keeps invalid UTF-8 text and drops non-XML characters
Package fix: deterministic ZIP output (fixed entry timestamps)
feat: DocumentBuilder::coreProperties()
Paragraphs and runs feat: arbitrary paragraph style id (w:pStyle)
feat: w:pageBreakBefore and w:widowControl on paragraphs
feat: East Asian font (w:eastAsia) on run styles
Tables fix: split a spanning cell's width across its grid columns
feat: table indent (w:tblInd)
feat: exact row height (w:hRule="exact")

All new constructor parameters are appended at the end, so positional callers are unaffected. isEmpty() and copy() cover the new fields. The CHANGELOG has one [Unreleased] section, and the README is updated in all four languages wherever the public API changed.

1. Run text

Tabs and newlines. renderRun() put all of Run::$text into one <w:t>, where Word shows a "\t" or "\n" as a space at best. The library hit this on its own round trips. BodyReader reads <w:tab/> as "\t", so a document read and written back lost every tab. Converter keeps a <pre>'s newlines in the run text, so preformatted HTML came out as one line. The writer now splits the text on "\t", "\r\n", "\r" and "\n", and writes <w:tab/> and <w:br/> between <w:t> segments in the same <w:r>:

<!-- new Run("Name:\tValue") -->
<w:r><w:t xml:space="preserve">Name:</w:t><w:tab/><w:t xml:space="preserve">Value</w:t></w:r>

Empty segments are left out. Text without tabs or newlines, and an empty run, give the same output as before. The table caption uses the same helper.

Escaping. XmlEscape had two problems:

  • It called htmlspecialchars() without ENT_SUBSTITUTE, so a string with one invalid UTF-8 byte became ''. The whole run text, hyperlink target or font name disappeared without a warning. The bad sequence now becomes U+FFFD, and the rest of the string is kept.
  • Characters outside the XML 1.0 Char production (C0 controls other than tab, LF and CR, plus U+FFFE and U+FFFF) were written as they were, and Word refused to open the file. They are now removed. They have no visible form, so a stray \x0B from a paste shouldn't turn into a replacement glyph.

This applies to text() and attr() alike, so relationship targets, style names, lvlText and core.xml are covered too.

2. Reproducible bytes and core properties

Deterministic ZIP. Identical input produced identical XML, but ZipArchive stamped every entry with the current time. Two writes more than one 2-second DOS tick apart therefore differed, which breaks checksums, cache keys, snapshot tests and diffs of generated files. Every entry now carries CoreProperties::$modified, read as a UTC wall-clock time. When that isn't set, it carries 1980-01-01 00:00:00, the earliest DOS time. Values outside 1980–2107 are clamped.

ZipArchive::setMtimeName() alone does not make the bytes match across machines. libzip 1.11.4 converts the timestamp with C localtime(), which reads the process's TZ. PHP's date.timezone makes no difference, and neither does putenv('TZ=UTC') before close(). setMtimeName($name, 315532800) writes:

TZ DOS time / date
UTC 0000 / 0021 (1980-01-01 00:00)
Asia/Tokyo 4800 / 0021 (09:00)
America/New_York 9800 / 019f (19:00 on an invalid "1980-12-31")

libzip writes no extended-timestamp or NTFS extra fields. The DOS time and date in the local header and in the central directory record are the only places the time is stored, and nothing else in the headers varied between runs. So Writer\ZipTimestamp::apply() overwrites those two fields for each entry after close(), walking the central directory from the end record. The CRC doesn't cover them, and ZipArchive::open(..., CHECKCONS) accepts the result. An archive it can't read safely (ZIP64, a comment, an unexpected signature) is returned unchanged; the writer never produces one. sha1(DocumentBuilder::new()->paragraph('Hi')->toBytes()) is now the same under TZ=UTC, Asia/Tokyo and America/New_York. ZipTimestamp is a final class with one static method. It is public only because it lives in the writer's namespace, so it could be marked @internal.

DocumentBuilder::coreProperties(). build() never set Document::$coreProperties, so builder users had to construct the Document by hand to get a title or author:

DocumentBuilder::new()
    ->coreProperties(new CoreProperties(
        title: 'Invoice #42',
        creator: 'Acme Inc.',
        modified: new DateTimeImmutable('2026-08-06 09:30:00', new DateTimeZone('UTC')),
    ))
    ->paragraph('Body')
    ->toFile('invoice.docx');   // every entry is stamped 2026-08-06 09:30:00

A caller who wants real timestamps passes modified: new DateTimeImmutable().

3. Paragraphs and runs

new Paragraph($runs, styleId: 'Quote');
$doc->paragraph(fn ($p) => $p->styleId('Quote')->pageBreakBefore()->text('...'));
new ParagraphStyle(pageBreakBefore: true, widowControl: false);
new RunStyle(fontFamily: 'Calibri', fontFamilyEastAsia: 'SimSun');
  • Paragraph::$styleId. A paragraph could reference only Heading1..6, through headingLevel. Styles registered with StyleRegistry::paragraph() could be defined but never applied. The id is written as <w:pStyle> and XML-escaped. An explicit id wins over headingLevel, and list items keep ListParagraph. The reader doesn't fill it in: it resolves style formatting into the paragraph's own styles, and the writer's registry may not contain the source document's styles.
  • pageBreakBefore. Until now the only page break was a PageBreak block, which writes a paragraph of its own and leaves an empty line at the top of the new page. The reader already parsed w:pageBreakBefore but dropped it before the model, so this also fixes a round-trip loss. HTML output is break-before: page, and break-before: page and page-break-before: always are parsed back.
  • widowControl is a ?bool. null writes nothing, true writes <w:widowControl/>, and false writes w:val="0". Because copy() treats null as "keep", it cannot reset the value to null. There is no CSS mapping, because widows and orphans are counts, not an on/off switch.
  • w:pPr order follows CT_PPrBase: pStyle, keepNext, keepLines, pageBreakBefore, widowControl, numPr, pBdr, …
  • fontFamilyEastAsia is written as w:eastAsia on <w:rFonts>, both in document runs and in styles.xml. With only fontFamily set, the output is byte-identical to before. The reader reads an explicit w:eastAsia. eastAsiaTheme isn't resolved, because the theme map holds only the Latin fonts.

4. Tables

$builder->table(fn (TableBuilder $t) => $t
    ->indentPt(18)                     // also indent(int $twips), indentMm()
    ->row(fn (TableRowBuilder $r) => $r->exactHeight(300)->cell('Fixed'))
    ->row(fn (TableRowBuilder $r) => $r->height(300)->cell('At least'))
);
new TableStyle(indentTwips: 360);
new TableRow($cells, heightTwips: 300, heightRule: RowHeightRule::Exact);
  • Grid fix. Without Table::$gridColumnsTwips, computeGridColumns() repeated a spanning cell's full width for every column it spans. A 2000-twip gridSpan: 2 cell next to a 2000-twip cell gave 2000 / 2000 / 2000, i.e. 6000 twips for a 4000-twip row. Widths also came from the first row only, so a title row spanning the table set every column (5000 over 1500 + 3500 gave 5000 / 5000). Now each column takes its width from the first row where an unspanned cell with a width covers it. Failing that, it takes a spanning cell's width divided by its span, and failing that, 2000 as before. The number of columns still comes from the first row. Tables with explicit grid columns are unaffected.
  • TableStyle::$indentTwips is written as <w:tblInd w:type="dxa"> at its place in CT_TblPr, between jc and tblBorders. Negative values are allowed. The reader reads it back. In HTML it is written as margin-left on the <table>, except for a centred table, and TableStyleApplier parses a length margin-left back. As a result, HTML that already set margin-left on a table now gets an indented table in Word.
  • TableRow::$heightRule is a backed enum, RowHeightRule::AtLeast (the default) or RowHeightRule::Exact, like VerticalAlign. Before, every w:trHeight was atLeast. The reader maps exact to Exact, and atLeast, auto or a missing rule to AtLeast. TableRowBuilder::height() resets the rule to AtLeast, so whichever of height() and exactHeight() is called last wins. The serializer doesn't write row heights today, so the rule isn't serialized either.

Not included

  • Style-level (styles.xml) line spacing, caps and outlineLvl, and a configurable docDefaults (font, size, w:lang). These touch how StyleRegistry is designed, so we'd rather open an issue first.
  • Two existing gaps we noticed but left alone:
    • The HTML style appliers build a fresh ParagraphStyle/RunStyle and drop several base fields (lineSpacingRule, keepWithNext, keepLines, caps).
    • ThemeResolver stores the minor-font alias as minorHAnsi, but resolveFont looks keys up in lowercase.

Tests

composer install && vendor/bin/phpunit: OK (468 tests, 1255 assertions). Before: OK (420 tests, 1068 assertions). composer stan: no errors. Each commit passes on its own: 429, 434, 440, 442, 446, 451, 455, 458, 464 and 468 tests.

Notable tests:

  • tests/Writer/RunTextTest.php, with a DocxReader → Word2007Writer round trip.
  • tests/Writer/XmlEscapeTest.php: document.xml and .rels parse with DOMDocument even with \x01 and invalid UTF-8 in the input.
  • tests/Writer/DeterministicOutputTest.php reads the DOS fields from the raw headers. statName() would convert through the local time zone and hide the difference. The test needs no sleep() and passes under all three time zones above.
  • The two grid tests fail on the old code.

🤖 Generated with Claude Code

serpentblade and others added 10 commits October 20, 2026 08:56
Run text went into a single <w:t> as is, so a "\t" or "\n" ended up
inside it, where Word shows it as a space at best. The reader turns
<w:tab/> into "\t", so a document read and written back lost its tabs,
and the converter keeps the newlines of a <pre>, which then lost its
line breaks.

The writer now splits the text on "\t", "\r\n", "\r" and "\n" and
writes <w:tab/> / <w:br/> between <w:t> segments in the same <w:r>,
leaving out empty segments. This covers runs anywhere (body, headers,
footers, hyperlinks, lists, cells) and the table caption.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
htmlspecialchars without ENT_SUBSTITUTE returns '' for a string with an
invalid UTF-8 sequence, so the whole run text or attribute value was
silently lost. And C0 control characters other than tab, newline and
carriage return passed through, which makes the part malformed: Word
refuses to open the file.

text() and attr() now escape with ENT_SUBSTITUTE (an invalid sequence
becomes U+FFFD) and remove every character outside the XML 1.0 Char
production (C0 controls, U+FFFE, U+FFFF).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZipArchive stamped every entry with the current time, so two writes of
the same document differed once a 2-second DOS tick passed. Every entry
now carries CoreProperties::$modified as a UTC wall-clock time, or
1980-01-01 00:00:00 when it is not set.

ZipArchive::setMtimeName() is not enough on its own: libzip converts the
Unix timestamp to a DOS date/time with the C localtime() and the
process's TZ (not PHP's date.timezone, and putenv('TZ=...') before
close() does not change it), so one timestamp still gave different
bytes on machines in different zones. ZipTimestamp writes the DOS
time/date fields into each local file header and central directory
record after close(). libzip writes no extended-timestamp extra field,
and the CRC does not cover these fields. An archive it cannot parse
(ZIP64, comment) is returned unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DocumentBuilder::build() never set Document::$coreProperties and the
builder had no way to pass them, so docProps/core.xml (title, author,
dates) was only reachable by building the Document by hand.
coreProperties(CoreProperties) stores them and build() passes them on.
The README documents it (in all four languages) along with the fixed
entry time from the previous commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paragraph::$styleId (appended to the constructor) and
ParagraphBuilder::styleId() write any paragraph style id as <w:pStyle>:
Normal, Title, Quote, or a custom id registered with
StyleRegistry::paragraph(). Before, only Heading1..6 could be referenced
(through headingLevel). An explicit id wins over the heading level; list
items keep ListParagraph. The id is XML-escaped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ParagraphStyle::$pageBreakBefore and ParagraphStyle::$widowControl
(?bool: null writes nothing, false writes w:val="0") are written in
CT_PPrBase order (after keepLines, before numPr), read back by
OoxmlPropertyParser/StylesResolver, and available on ParagraphBuilder.
The reader already parsed pageBreakBefore but dropped it on the way into
the model, so a paragraph that started a new page lost its break on a
read/write round trip.

HTML: pageBreakBefore serializes as break-before: page; the converter
parses break-before: page and page-break-before: always. widowControl has
no HTML mapping.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RunStyle::$fontFamilyEastAsia (appended to the constructor), with
withFontFamilyEastAsia() and RunStyleBuilder::fontFamilyEastAsia(), is
written as w:eastAsia on <w:rFonts>, in document runs and in style run
properties (a shared RunFonts helper). <w:rFonts> is written when either
font is set; with only fontFamily the output is unchanged. The reader
keeps an explicit w:eastAsia in its own slot; an eastAsiaTheme reference
is not resolved (the theme map holds the Latin faces only).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
computeGridColumns() repeated a spanning cell's full width for each
column it spans, so a 2000-twip cell with gridSpan 2 produced two
2000-twip gridCols and the grid was wider than the cell. The spanned
columns now share the width (width / span).

The grid was also taken from the first row only. A column now takes its
width from the first row where an unspanned cell with a known width
covers it, falling back to a spanning cell's share, then to 2000 twips.
A title row spanning the whole table no longer decides the columns of
the rows below it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TableStyle::$indentTwips and TableBuilder::indent() / indentMm() /
indentPt() write <w:tblInd w:w=".." w:type="dxa"/> in CT_TblPr order
(after jc, before tblBorders). TableReader parses a dxa tblInd back;
nil and auto read as no indent. The HTML serializer writes it as
margin-left on the <table> (unless the table is centred), and the
converter reads a length margin-left back as the indent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The writer wrote every row height as w:hRule="atLeast". TableRow gains
$heightRule (RowHeightRule::AtLeast, the default, or ::Exact), written as
w:hRule, and TableRowBuilder::exactHeight(). TableReader reads
hRule="exact" back; atLeast, auto and a missing hRule read as AtLeast,
as before. The rule is carried through the reader's and the HTML
converter's row-span reconstruction.

The HTML serializer does not write row heights today, so the rule is not
serialized either.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dskripchenko added a commit that referenced this pull request Oct 27, 2026
Writer fidelity (from #7) with follow-up fixes
@dskripchenko
dskripchenko merged commit 2c8d820 into dskripchenko:main Oct 27, 2026
8 checks passed
@dskripchenko

dskripchenko commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Thank you, @serpentblade — this was a careful, well-researched contribution. The libzip/TZ analysis behind the deterministic ZIP output in particular is something we would not have found on our own.

All ten of your commits are merged unchanged, with your authorship, through #8: GitHub would not let us push to this branch, so we carried them over to a branch here and added four follow-up commits on top.

What we changed, and why:

  • ZipTimestamp in linear time. substr_replace() per entry copied the whole archive twice per entry, so a document with 2,000 images over 60 MB took about 35 s and ~250 MB. It now collects the offsets first and writes the archive once. We also added bounds checks (a truncated central directory leaves the archive unchanged instead of warning) and marked the class @internal, as you suggested.
  • \v and \f become <w:br/> and <w:br w:type="page"/>. In text pasted from Word they are a manual line break and a page break; dropping them glued words together. Other non-XML characters are still removed, as you had it.
  • CHANGELOG, Changed: ZIP entry times now default to 1980-01-01, and HTML tables with margin-left get w:tblInd.
  • README in all four languages: styleId() only takes effect for a style the document defines.

A small request for next time: please tick "Allow edits by maintainers" when opening a pull request. For a fork owned by an organisation GitHub may still refuse, so a fork under your personal account works best. Then we can push small fixes straight into your branch and keep the conversation in one place.

Merged through #8. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants