860180 - U+30D8 causes XML error in UTF-16BE

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ryan Richards]

Attachment: U+30D8 in a minimal XML file

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:20.0) Gecko/20100101 Firefox/20.0
Build ID: 20130326150557

Steps to reproduce:

After upgrading to version 20 I navigated to http://アニメイヘム.com/


Actual results:

I received an XML Parsing Error: not well-formed


Expected results:

The document is well formed and should have displayed.

Comment 1

[Matthias Versen [:Matti]]

Last good nightly: 2012-11-08
First bad nightly: 2012-11-09

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=36e99ea02c05&tochange=90cea19e27e2

bug 801402 is most likely the culprit here.
I bisect this if required

Comment 2

[Masatoshi Kimura [:emk]]

Similar to 844007 which is supposed to be fixed in Firefox 20. However the page encoding is actually utf-16 (but big endian) here...

Comment 3

[Ryan Richards]

30

Comment 4

[Ryan Richards]

Checking other characters, looks like every U+nnD8 to U+nnDF character causes the error.

Comment 5

[bhavana bajaj [:bajaj]]

Tracking and passing this on to :emk as this is a regression from 801402. Will be very helpful to have a patch uplifted before Beta 3/Beta 4 goes to build.

Comment 6

[Masatoshi Kimura [:emk]]

This is because the "UTF-16" label no longer represents a sniffing decoder. It is an alias of "UTF-16LE" now. I'm working on a fix.

Comment 7

[Masatoshi Kimura [:emk]]

Attachment: Prefer UTF-16BE/LE to UTF-16

This patch will basically undo bug 335531. The Encoding Standard intentionally "misuses" the UTF-16 label.

Comment 8

[Masatoshi Kimura [:emk]]

https://tbpl.mozilla.org/?tree=Try&rev=bca208240bda

Comment 9

[Masatoshi Kimura [:emk]]

Attachment: Regression tests

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: Regression tests, v2

Forgot to call revokeObjectURL().

Comment 11

[Ryan Richards]

Just a bit of a follow up to the intial report,

U+30D7,プ, when swapped to D730 is 휰, a valid code point.
U+30D8,ヘ, when swapped to D830 is an invalid code point.
U+30D7 displays properly as プ while U+30D8 errors out.

Comment 12

[Henri Sivonen (:hsivonen)]

Comment on attachment 737026 [details] [diff] [review]
Regression tests, v2

Did we establish somewhere that Web compat prevents treating bogus labeling as fatal? Did that get written up in a spec somewhere?

Comment 13

[Masatoshi Kimura [:emk]]

(In reply to Henri Sivonen (:hsivonen) from comment #12)
> Comment on attachment 737026 [details] [diff] [review]
> Regression tests, v2
> 
> Did we establish somewhere that Web compat prevents treating bogus labeling
> as fatal? Did that get written up in a spec somewhere?

We need a self-contained encoding sniffing algorithm for XML because the XML spec assumes that the "UTF-16" can represent both endians.

Comment 14

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e58f99cad251
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d40dce142e0

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e58f99cad251
https://hg.mozilla.org/mozilla-central/rev/5d40dce142e0

Comment 16

[bhavana bajaj [:bajaj]]

(In reply to Masatoshi Kimura [:emk] from comment #14)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/e58f99cad251
> https://hg.mozilla.org/integration/mozilla-inbound/rev/5d40dce142e0

Can you please request nomination for aurora/beta here ? Beta 4 which goes to build tomorrow will be the last opportunity to land speculative low risk fixes.

Comment 17

[Masatoshi Kimura [:emk]]

Comment on attachment 736323 [details] [diff] [review]
Prefer UTF-16BE/LE to UTF-16

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 801402
User impact if declined: Some websites will not be displayed at all.
Testing completed (on m-c, etc.): on m-c in a few days.
Risk to taking this patch (and alternatives if risky): low 
String or IDL/UUID changes made by this patch: none

Comment 18

[Masatoshi Kimura [:emk]]

Comment on attachment 737026 [details] [diff] [review]
Regression tests, v2

[Approval Request Comment]
See above.

Comment 19

[bhavana bajaj [:bajaj]]

Comment on attachment 736323 [details] [diff] [review]
Prefer UTF-16BE/LE to UTF-16

The patch is low risk and an Fx20 regression which does not display a few websites.
Please make sure to land on mozilla-beta asap, as we are going to build today with Fx21 beta 4.

Comment 20

[bhavana bajaj [:bajaj]]

(In reply to Ryan Richards from comment #0)
> Created attachment 735628 [details]
> U+30D8 in a minimal XML file
> 
> User Agent: Mozilla/5.0 (Windows NT 6.0; rv:20.0) Gecko/20100101 Firefox/20.0
> Build ID: 20130326150557
> 
> Steps to reproduce:
> 
> After upgrading to version 20 I navigated to http://アニメイヘム.com/
> 
> 
> Actual results:
> 
> I received an XML Parsing Error: not well-formed
> 
> 
> Expected results:
> 
> The document is well formed and should have displayed.

Thanks for the report ! Can you try our latest nightly or aurora when the patch is landed, to confirm the issue is fixed for you ? Thanks

Comment 21

[Masatoshi Kimura [:emk]]

Could you also approve attachment 738720 [details] [diff] [review]? Otherwise this patch will cause permaorange on Thundirbird tinderboxes.
If not, I'll withdraw requesting beta approval.

Comment 22

[bhavana bajaj [:bajaj]]

(In reply to Masatoshi Kimura [:emk] from comment #21)
> Could you also approve attachment 738720 [details] [diff] [review]?
> Otherwise this patch will cause permaorange on Thundirbird tinderboxes.
> If not, I'll withdraw requesting beta approval.

Who would be able to review this ?

Comment 23

[Masatoshi Kimura [:emk]]

(In reply to bhavana bajaj [:bajaj] from comment #22)
> (In reply to Masatoshi Kimura [:emk] from comment #21)
> > Could you also approve attachment 738720 [details] [diff] [review]?
> > Otherwise this patch will cause permaorange on Thundirbird tinderboxes.
> > If not, I'll withdraw requesting beta approval.
> 
> Who would be able to review this ?

It's already reviewed by Simon and even landed (as I wrote in the approval request).

Comment 24

[bhavana bajaj [:bajaj]]

(In reply to Masatoshi Kimura [:emk] from comment #23)
> (In reply to bhavana bajaj [:bajaj] from comment #22)
> > (In reply to Masatoshi Kimura [:emk] from comment #21)
> > > Could you also approve attachment 738720 [details] [diff] [review]?
> > > Otherwise this patch will cause permaorange on Thundirbird tinderboxes.
> > > If not, I'll withdraw requesting beta approval.
> > 
> > Who would be able to review this ?
> 
> It's already reviewed by Simon and even landed (as I wrote in the approval
> request).

I was just getting to 863025, its approved now :)

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e36ec18f4854
https://hg.mozilla.org/releases/mozilla-aurora/rev/940312febd4b

https://hg.mozilla.org/releases/mozilla-beta/rev/a1b7febaccf0
https://hg.mozilla.org/releases/mozilla-beta/rev/8df81abcb329

Comment 26

[Ryan Richards]

(In reply to bhavana bajaj [:bajaj] from comment #20)
> (In reply to Ryan Richards from comment #0)
> > Created attachment 735628 [details]
> > U+30D8 in a minimal XML file
> > 
> > User Agent: Mozilla/5.0 (Windows NT 6.0; rv:20.0) Gecko/20100101 Firefox/20.0
> > Build ID: 20130326150557
> > 
> > Steps to reproduce:
> > 
> > After upgrading to version 20 I navigated to http://アニメイヘム.com/
> > 
> > 
> > Actual results:
> > 
> > I received an XML Parsing Error: not well-formed
> > 
> > 
> > Expected results:
> > 
> > The document is well formed and should have displayed.
> 
> Thanks for the report ! Can you try our latest nightly or aurora when the
> patch is landed, to confirm the issue is fixed for you ? Thanks

Open my site in Nightly, everything is working.

Comment 27

[u279076]

Are there any user facing regression risks which QA should look at not covered by existing tests?

Comment 28

[Masatoshi Kimura [:emk]]

Unlikely, hence low risk.

Comment 29

[u279076]

(In reply to Masatoshi Kimura [:emk] from comment #28)
> Unlikely, hence low risk.

Okay, thanks Masatoshi. I just wanted to confirm before flagging this [qa-].