Repository navigation
Draw an inline object that follows a U+FEFF that leaves no text - #871
Draft
gravitypdf-bot wants to merge 3 commits into
Draft
gravitypdf-bot wants to merge 3 commits into
gravitypdf-bot wants to merge 3 commits into
Conversation
A text node whose only content was U+FEFF saved no chunk and left its empty run on $this->OTLdata, where the next saved chunk picked it up. printbuffer() then took that run for the chunk's own, so an image, barcode, input, dottab, hr, meter or textcircle never got its U+FFFC and was not drawn, and a br was lost. The run now ends with its text node. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…text node Only the text-node branch of WriteHTML() gives OTLdata a value, and it now clears it at its end, so the tag classes and _saveTextBuffer() never find one. _saveCellTextBuffer() keeps its reset: a text node after a block in a cell saves a line break first, which takes the run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On newer versions of PHP the UTF-32 round trip in is_utf8() drops a leading U+FEFF, so valid UTF-8 starting with a byte order mark threw as invalid. mb_check_encoding() asks the question directly. U+FEFF reaching a core font would be drawn as a question mark, and through a direct call as a missing glyph, so it is now dropped there as HTML text in a Unicode font already did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
|
What's the overall performance improvement for a large document? |
jakejackson1
marked this pull request as draft
November 6, 2026 08:34
4 tasks
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.
Fixes #870.
Problem
A U+FEFF that leaves no text made the next inline object disappear with no error. Reproduced on
gravitypdf:<style>…</style>+ U+FEFF +<img>draws no image. The same happens with barcode, input,<dottab>,<hr>,<meter>and<textcircle>, and a<br>in that position is lost too, so the following text sits a line too high.$this->OTLdatais set for a text node, then the U+FEFF is removed from it. If no text remains, no chunk is saved, so the leftover empty run goes to the next chunk that is saved.printbuffer()then treats that run as the chunk's own and doesn't give an object its U+FFFC. U+FEFF also turnsbiDirectionalon, soBidi::prepare()runs over the object's empty run and the object never reachesWriteFlowingBlock().Fix
Clear
$this->OTLdataat the end of the text-node branch inWriteHTML(). That is the only place it gets a non-empty value, and nocontinuecan skip the reset, so a run can't outlive its text node. This also makes the reset in the select branch redundant, so that line is removed.The issue proposed checking
char_datain both save functions. I used a single reset because it needs one edit and the save functions don't have to know the shape of a run. I didn't run the issue's proposal on this branch.Tests
tests/Mpdf/ByteOrderMarkTest.php: 20 documents, each compared with the same document without the U+FEFF in a Unicode font and a core one. It covers each object type, the placements in the issue and a mark at the very start. It also covers direct text calls with the mark at each end and in the middle, and that invalid UTF-8 still throws, with and without a leading mark.ByteOrderMarkBeforeObjectSnapshotTest: a captioned fixture. I rasterised it and looked at it: every object appears under its caption. On the unfixed code all of them are missing and the table cell is empty.The original 14 unit cases and the snapshot fail on
gravitypdf. With only the first commit in place, 26 of the 30 unit cases and the snapshot fail or error; the 4 that pass are the invalid UTF-8 cases, which should pass either way.Run locally (PHP 8.5)
composer test: 13551 tests pass, 31 skipped. The snapshot group on its own (258 tests) has no skips.composer cs: clean.src/Mpdf.php, on lines this PR doesn't touch.Not run
A byte order mark at the very start
A U+FEFF at the very start of the HTML never reached the bug above, because
WriteHTML()threw "HTML contains invalid UTF-8 character(s)" first.is_utf8()compared the string with its round trip through UTF-32, and on newer versions of PHP that round trip drops a leading mark. It does on 8.2, 8.3 and 8.5, and doesn't on 7.4; 8.0, 8.1 and 8.4 weren't run.is_utf8()and the loop inpurify_utf8()now usemb_check_encoding(). Over 400,000 random and edge strings on each of those four versions, it disagreed with the round trip only on a leading mark.is_utf8()is also much faster: about 3 µs down to 23 ns per KB of valid text, from a micro-benchmark on 8.5.Accepting the mark meant it had to not be drawn either:
utf8ToWin1252()drops U+FEFF, which windows-1252 would turn into?. That coversWriteHTML()in a core font, at any position.purify_utf8_text()drops it at any position, as HTML text in a Unicode font already did. That coversWriteCell(),WriteText(),MultiCell()and the other calls that use it.Write()andCell()don't validate their text and never threw. They still draw the mark, as they do today.So a U+FEFF in the middle of text given to one of those direct calls, or to a core font, is now dropped rather than drawn. The
ignore_invalid_utf8branch keeps its round trip.Removing the resets
The text-node reset makes the other
OTLdataresets redundant, so the seven inTag.php,BlockTag.phpandInlineTag.phpand the one in_saveTextBuffer()are gone. Output is unchanged:composer testpasses with no fixture refreshed. Three stay:_saveCellTextBuffer(): a text node after a block in a table cell saves a line break first, so this reset is what stops both chunks taking the run.$this->OTLdata = nullat the top of the text branch:MultiCell()branches onis_array($OTLdata), so a textarea would behave differently with[].Related, not changed here
<p>\xE2, HTML ending in a truncated sequence, hangspurify_utf8(). It's ongravitypdftoo and is being fixed separately. That fix touches the same loop as this PR.