1749522 - Plain text tokenized as HTML upon encoding rewind

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

1. Open view-source:https://bug1749522.bmoattachments.org/attachment.cgi?id=9258559
2. See if anything is visible

DevTools shows that <script> is somehow not escaped properly, but doesn't seem that it runs any script.

Comment 1

[Masatoshi Kimura [:emk]]

Even non view-source URL shows nothing for me.

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

That's expected, it only includes a script.

Edit: Oh wait, it's served with text/plain, so it should show something!

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: test

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: testcase.html

Comment 5

[Masatoshi Kimura [:emk]]

(In reply to Kagami :saschanaz from comment #2)

Edit: Oh wait, it's served with text/plain, so it should show something!

Yes, Firefox 95.0.2/96 shows the HTML source.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1701828

Comment 7

[Christian Holler (:decoder)]

NI based on regressor. This sounds somewhat serious and we probably don't want to ship with this.

Comment 8

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1749522 - When plain text encoding speculation fails, restart the plaintext mode of the tokenizer.

Comment 9

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=317a739418b0fee1c2673332b3cad0c96886ab8e

(In reply to Christian Holler (:decoder) from comment #7)

NI based on regressor. This sounds somewhat serious and we probably don't want to ship with this.

Indeed, we should uplift to beta.

Comment 10

[Henri Sivonen (:hsivonen)]

[Tracking Requested - why for this release]:
Bad regression for plain text that contains HTML tag-like text (not just plain text view source) in some scenarios where there is no (usable) encoding declaration. (The test case here declares IBM855, which is an unsupported label.)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2d8eae8d006
When plain text encoding speculation fails, restart the plaintext mode of the tokenizer. r=smaug

Comment 12

[Sandor Molnar[:smolnar]]

Backed out for causing reftest failures in tests/reftest/bug1749522-1

Backout link: https://hg.mozilla.org/integration/autoland/rev/ff88218a0ce8ec0cd8fb9332f73ee4156bf20a4a

Push with failures

Failure log

Comment 13

[Henri Sivonen (:hsivonen)]

(In reply to Sandor Molnar from comment #12)

Backed out for causing reftest failures in tests/reftest/bug1749522-1

Backout link: https://hg.mozilla.org/integration/autoland/rev/ff88218a0ce8ec0cd8fb9332f73ee4156bf20a4a

Push with failures

Failure log

The result is that the test fails intermittently on Android and consistently passes elsewhere. This is cross-platform code, so there isn't any mechanism why this would sometimes fail on Android while sometimes passing on Android and always passing on desktop.

I suspect an Android reftest harness bug.

Comment 14

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5732eb21a8180ea01d1fa1de8f95bf2903b1c65f

Comment 15

[Henri Sivonen (:hsivonen)]

Relanding with test skipped on Android.

Comment 16

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #13)

I suspect an Android reftest harness bug.

Filed as bug 1750511.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37009356a0c3
When plain text encoding speculation fails, restart the plaintext mode of the tokenizer. r=smaug

Comment 18

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/37009356a0c3

Comment 19

[Henri Sivonen (:hsivonen)]

Comment on attachment 9258859 [details]
Bug 1749522 - When plain text encoding speculation fails, restart the plaintext mode of the tokenizer.

Beta/Release Uplift Approval Request

• User impact if declined: Plain text content disappears in whole or part if it doesn't have an encoding declaration and has content that resembles HTML markup, notably less-than sign followed by an ASCII letter or exclamation mark.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Not risky due to the mechanism of the bug and fix being well understood.
• String changes made/needed: None

Comment 20

[Donal Meehan [:dmeehan]]

Comment on attachment 9258859 [details]
Bug 1749522 - When plain text encoding speculation fails, restart the plaintext mode of the tokenizer.

Approved for 97.0b5. Thanks.

Comment 21

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f9bcb0a705cd

Comment 22

[Peter Magyari (Desktop QA)]

I've reproduced the issue on Firefox 97.0b4 and verified the fix using Nightly 98.0a1 and Beta 97.0b5 on Windows 10, MacOS 11.6 and Ubuntu 20.04.