Repository navigation
Writer fidelity (from #7) with follow-up fixes - #8
Merged
Merged
Conversation
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>
ZipTimestamp::apply() called substr_replace() on the whole archive twice per entry, so a package with many media parts took time proportional to entries × size: 2000 entries of 8 KB took about 4 s, 2000 entries in a 60 MB package about 35 s. It now collects the offsets of the time fields while walking the central directory and writes the archive out in one pass. A central directory record has to fit before the end record and a local file header inside the string; otherwise the archive is returned unchanged instead of reading past the end. The class is marked @internal: it is a helper of Word2007Writer. Tests: a 2000-entry archive within a time budget, a truncated central directory, and identical bytes under TZ=UTC, Asia/Tokyo and America/New_York (in child processes, since libzip reads TZ at start). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
XmlEscape drops characters outside the XML 1.0 Char production, so a vertical tab or a form feed in run text disappeared and glued the words on either side together. Word's object model and clipboard use exactly these two for a manual line break and a page break, so text taken from Word lost its breaks. renderRunContent() now splits on them too: "\v" becomes <w:br/> and "\f" <w:br w:type="page"/>. Other non-XML characters are still dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both change the output for existing callers, so they belong under Changed and not only under Added/Fixed: entries are stamped with CoreProperties::$modified or 1980-01-01 instead of the write time, and an HTML table with a length margin-left now gets <w:tblInd>. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The README example writes styleId('Quote'), but Quote is not in the
default registry, and Word shows a paragraph with an unknown style id as
Normal. One line in all four READMEs says which ids exist by default and
that others have to be registered with StyleRegistry::paragraph().
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This carries the ten commits of #7 by @serpentblade unchanged, authorship kept, and adds four follow-up commits from the review. Many thanks to the author for a careful, well-documented contribution — the reasoning in #7 made the review easy.
The follow-ups:
ZipTimestampin linear time. Patching the archive withsubstr_replace()per entry copied the whole archive twice per entry: 2,000 entries over 60 MB took 35 s and ~250 MB. The offsets are now collected first and the archive is written once. Records that don't fit ($pos + 46 > $eocd,$local + 30 > strlen) leave the archive unchanged instead of warning on a truncated input, and the class is marked@internal, as Writer fidelity: run text, escaping, reproducible bytes, paragraph and table properties #7 itself suggested. Tests: a 2,000-entry archive within a time budget, a truncated central directory, identical bytes under threeTZvalues in child processes.\vand\fin run text become a line break and a page break. In text pasted from Word they mean exactly that; dropping them glued words together ("line2\x0Bvt"→line2vt). Other non-XML characters are still removed, as in Writer fidelity: run text, escaping, reproducible bytes, paragraph and table properties #7.CoreProperties::$modifiedis not set; HTML tables with a lengthmargin-leftnow getw:tblInd.styleId()only takes effect for a style the document defines (built-in headings,ListParagraph,Caption, or one registered withStyleRegistry::paragraph()).phpunit 472 tests, PHPStan clean, ECMA-376 XSD validation of the conformance document and of a document exercising every new element, LibreOffice and python-docx open the output.
Closes #7.
🤖 Generated with Claude Code