Skip to content

Draw an inline object that follows a U+FEFF that leaves no text - #871

Draft
gravitypdf-bot wants to merge 3 commits into
gravitypdffrom
fix/870-feff-inline-object
Draft

gravitypdf-bot wants to merge 3 commits into
gravitypdffrom
fix/870-feff-inline-object

Conversation

@gravitypdf-bot

@gravitypdf-bot gravitypdf-bot commented Nov 6, 2026 •

Copy link
Copy Markdown

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->OTLdata is 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 turns biDirectional on, so Bidi::prepare() runs over the object's empty run and the object never reaches WriteFlowingBlock().

Fix

Clear $this->OTLdata at the end of the text-node branch in WriteHTML(). That is the only place it gets a non-empty value, and no continue can 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_data in 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.
  • PHPStan 2.2.17 (not CI's pinned version): no errors on the changed lines. It reports errors elsewhere in src/Mpdf.php, on lines this PR doesn't touch.

Not run

  • PHP 5.6 to 8.4 and Windows are left to CI.
  • Whether upstream mPDF has the same bug wasn't checked.

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 in purify_utf8() now use mb_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 covers WriteHTML() 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 covers WriteCell(), WriteText(), MultiCell() and the other calls that use it.
  • Write() and Cell() 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_utf8 branch keeps its round trip.

Removing the resets

The text-node reset makes the other OTLdata resets redundant, so the seven in Tag.php, BlockTag.php and InlineTag.php and the one in _saveTextBuffer() are gone. Output is unchanged: composer test passes 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.
  • The textarea branch: the run is already copied into the object, so the chunk must not carry it too.
  • $this->OTLdata = null at the top of the text branch: MultiCell() branches on is_array($OTLdata), so a textarea would behave differently with [].

Related, not changed here

  • Text after a block in a table cell loses its OpenType positioning. That line break takes the run, so the text is drawn without GPOS, and LTR-table Arabic in XB Riyaz comes out scrambled. This predates this PR and isn't fixed here.
  • <p>\xE2, HTML ending in a truncated sequence, hangs purify_utf8(). It's on gravitypdf too and is being fixed separately. That fix touches the same loop as this PR.

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>
@jakejackson1 jakejackson1 added bug Something isn't working create-upstream-pr labels Nov 6, 2026
gravitypdf-bot and others added 2 commits November 6, 2026 19:27
…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>
@jakejackson1

Copy link
Copy Markdown
Member

What's the overall performance improvement for a large document?

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

Labels

bug Something isn't working create-upstream-pr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A U+FEFF that leaves no text makes the next inline object disappear

2 participants