634541 - HTML parser should remove only one BOM

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Henri Sivonen (:hsivonen)]

Attachment: Test case

By code inspection, it seems that asking for a "UTF-16BE" or "UTF-16LE" decoder returns a decoder that has fixed endianness but still swallows the first character if it is a U+FEFF.

This is wrong, because only "UTF-16" should interpret U+FEFF as a BOM and the explicit LE and BE variant should treat it as a ZWNBSP.

This is bad, because code that has done its own BOM sniffing and wants a decoder without BOM swallowing state (the HTML5 parser) can't get one.

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Test case with doctype

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: Two BOMs, no explicit LE label

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Three BOMs no explicit label

Comment 4

[Henri Sivonen (:hsivonen)]

Chrome swallows one BOM as well. Opera 11 seems to swallow any number of BOMs. Let's assume the Web might require this and deal on the parser side.

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: plain text testcase

Comment 6

[Masatoshi Kimura [:emk]]

All browsers seem to swallow UTF-16LE BOM even for plain text.

Comment 7

[Anne (:annevk)]

Is this still a bug? Per the Encoding Standard the UTF-16 variants work a bit differently and since we now implement that...

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to Anne (:annevk) from comment #7)
> Is this still a bug?

The test cases with more than one BOM still demonstrate a bug: Two BOMs are removed from the start of the stream.

Comment 9

[Henri Sivonen (:hsivonen)]

Attachment: Bug 634541 - Make the HTML parser remove only one BOM when the input starts with multiple BOMs.

MozReview-Commit-ID: 1zoGFxx9MCm

Comment 10

[Henri Sivonen (:hsivonen)]

Let's see if there are already tests that test this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8aab8ed5a98fe52ed87698981ee0dbe1d1510526

Comment 11

[Henri Sivonen (:hsivonen)]

This patch leaves an unswallowed BOM at the start of reloaded document.open()ed docs. parser/htmlparser/tests/mochitest/test_bug715739.html

Comment 12

[Henri Sivonen (:hsivonen)]

Trying again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=305408dd3f44a79f9f052fba1d7ffd8c3d92021f

Comment 13

[Henri Sivonen (:hsivonen)]

WPT lacked a pre-existing test, so adding one:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a17225a6a347514f30ebad04cc012df3c41f0c7

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8998478 [details]
Bug 634541 - Make the HTML parser remove only one BOM when the input starts with multiple BOMs.

Boris Zbarsky [:bzbarsky, bz on IRC] (vacation Aug 16-27) has approved the revision.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aeb2e2eaf0c4
Make the HTML parser remove only one BOM when the input starts with multiple BOMs. r=bzbarsky

Comment 16

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/aeb2e2eaf0c4

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12454 for changes under testing/web-platform/tests

Comment 18

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged