Skip to content

LibWeb+LibTextCodec: Fix a few WPT encoding test failures#9610

Closed
Martin005 wants to merge 4 commits into
LadybirdBrowser:masterfrom
Martin005:encoding-fixes
Closed

LibWeb+LibTextCodec: Fix a few WPT encoding test failures#9610
Martin005 wants to merge 4 commits into
LadybirdBrowser:masterfrom
Martin005:encoding-fixes

Conversation

@Martin005

@Martin005 Martin005 commented May 24, 2026

Copy link
Copy Markdown
Contributor

Fixes some of the failing WPT subtests for the encoding test files:

  • Set document encoding for XML documents (fbffcec: 2 subtests)
  • Implement step 6 of encoding sniffing for parent document inheritance (41c1ac1: 3 subtests)
  • Support ignoreBOM in TextDecoder and TextDecoderStream (c947d45: 16 subtests)
  • Handle U+FFFD as unmappable in ISO-2022-JP encoder (e519ebe: 4 subtests)

@Martin005 Martin005 changed the title LibWeb+LibTextCodec: Fix 25 WPT encoding test failures LibWeb+LibTextCodec: Fix WPT encoding test failures May 24, 2026
@Martin005 Martin005 changed the title LibWeb+LibTextCodec: Fix WPT encoding test failures LibWeb+LibTextCodec: Fix a few WPT encoding test failures May 24, 2026

@sideshowbarker sideshowbarker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase this against current master. The CI runs are hitting timeouts for some stuff that’s subsequently been fixed on master.

Comment thread Libraries/LibWeb/DOM/DocumentLoading.cpp Outdated
Comment thread Libraries/LibWeb/DOM/DocumentLoading.cpp Outdated
@sideshowbarker

Copy link
Copy Markdown
Member

OK, the CI failures we’re getting here now suggest there’s a general test-harness bug that needs to be investigated and fixed (separate from this PR) before this PR could move ahead.

Comment thread Libraries/LibWeb/DOM/DocumentLoading.cpp Outdated
@Martin005
Martin005 force-pushed the encoding-fixes branch 2 times, most recently from b4322fe to 4b0e4af Compare May 30, 2026 15:39
@Martin005
Martin005 requested a review from AtkinsSJ May 31, 2026 07:55

@sideshowbarker sideshowbarker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good to me, though I still want to look at it more detail before approving. But I think it should also have review and sign-of from @AtkinsSJ

@sideshowbarker
sideshowbarker dismissed their stale review May 31, 2026 11:42

Suggested changes made.

@github-actions github-actions Bot added the conflicts Pull request has merge conflicts that need resolution label Jun 3, 2026
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest master.

Martin005 added 4 commits June 3, 2026 14:25
Once the character encoding is established for an XML document, the
document's character encoding must be set to that encoding. Previously,
the detected encoding was only used to select a TextCodec::Decoder and
was never stored in document.m_encoding, so doc.characterSet always
returned the default UTF-8.

This fixes two WPT subtests that expect utf-32-little-endian-bom.xml to
report characterSet as UTF-16LE when the BOM is misinterpreted.
Step 6 of the HTML encoding sniffing algorithm specifies that if the
parser is associated with a Document whose container document is
non-null and same-origin, the parent document's character encoding
should be used (unless it's UTF-16BE/LE).

This fixes three WPT subtests that expect UTF-32 HTML resources loaded
in iframes to inherit their parent document's UTF-8 encoding.

To ensure the document's navigable is available when encoding detection
runs (for synchronously available body bytes), the navigable is now
set on the document before starting the incremental HTML parser.
Plumb a strip_bom flag through Decoder::to_utf8 so TextDecoder and
TextDecoderStream can honor the ignoreBOM option. When ignoreBOM is
true, the BOM is preserved in decoded output.

This fixes 15 WPT subtests across textdecoder-ignorebom.any.js and
streams/decode-ignore-bom.any.js.

Use incomplete_tail_length for non-UTF-8 encodings in TextDecoderStream
to avoid splitting multi-byte sequences at chunk boundaries.
The jis0208 index contains 0xFFFD placeholder entries for unmapped
pointers. The generator-produced inverse accessor would match these
when encoding U+FFFD itself, returning a bogus JIS pointer instead of
reporting the code point as unmappable.

Guard at the call site in the ISO-2022-JP encoder rather than modifying
the Python generator, to avoid changing inverse accessors for other
indexes (big5, euc_kr, gb18030, etc.) which don't exhibit this bug.

This fixes 4 WPT subtests in iso-2022-jp-encoder.html.
@github-actions github-actions Bot removed the conflicts Pull request has merge conflicts that need resolution label Jun 3, 2026
@gmta

gmta commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Thank you for taking the time to work on this pull request.

As announced in "Changing How We Develop Ladybird", we are closing all currently open public pull requests as part of ending the public code contribution path for Ladybird.

This is not a judgment on the quality of your work; we are very grateful for the effort you put into this pull request. But as of right now, code changes to the Ladybird codebase will only be introduced by project maintainers.

We still welcome clear bug reports, reduced test cases, website testing, standards discussion, design discussion, security reports, and technical feedback.

@gmta gmta closed this Jun 5, 2026
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.

4 participants