959058 - Make gbk use the gb18030 decoder

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P4)

Description

[Henri Sivonen (:hsivonen)]

The gb18030 decoder is a superset of the gbk decoder, it's not particularly useful to the user to refuse to decode the gb18030 byte sequences when a page is labeled as gbk and the Encoding Standard already unifies the decoding.

See bug 951691 for why unify the encoder side, too.

Comment 1

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #0)
> See bug 951691 for why unify the encoder side, too.

Why *not to* unify the encoder side...

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: Remap the constructors, leave behind dead code

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

This patch  leaves behind dead code  in case we find a reason to reverse this change.

emk, do you prefer to get rid of the dead code at the time of landing this change or waiting and seeing if  this sticks on the release channel before removing the dead code?

Comment 3

[Masatoshi Kimura [:emk]]

I have no strong opinion about that. Feel free to choose whichever you like.

Comment 4

[Henri Sivonen (:hsivonen)]

Sigh. This consistently fails test_langpack.js. Unassigning from self in case someone else wants to figure out why while I work on a Q1 goal.

Comment 5

[Henri Sivonen (:hsivonen)]

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

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #5)
> https://tbpl.mozilla.org/?tree=Try&rev=f193028a6032

test_langpack.js fails still after rebasing, so it wasn't just random badness with the baseline revision. :-(

Comment 7

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #6)
> test_langpack.js fails still after rebasing, so it wasn't just random
> badness with the baseline revision. :-(

Except now I saw it in another try run that didn't involve the patch from this bug...

Comment 8

[Henri Sivonen (:hsivonen)]

So it looks like the patch from this bug is green to go if:
 1) Someone figures out what to attribute the test_langpack.js failure to.
 2) This patch gets a unit test of its own.

Comment 9

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #8)
>  1) Someone figures out what to attribute the test_langpack.js failure to.

Was unrelated.

>  2) This patch gets a unit test of its own.

I have a test.

Re-taking.

Comment 10

[Henri Sivonen (:hsivonen)]

Attachment: Remap the constructors, leave behind dead code, v2 with test

I'll file another bug about removing dead code.

Comment 11

[Henri Sivonen (:hsivonen)]

Thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/88549ccbce60

Comment 12

[Carsten Book [:Tomcat]]

backed out because of perma m7 orange on android like https://tbpl.mozilla.org/php/getParsedLog.php?id=33621409&tree=Mozilla-Inbound

Comment 13

[Henri Sivonen (:hsivonen)]

Caused by bug 962029.

Comment 14

[Henri Sivonen (:hsivonen)]

Attachment: Test 0x80 decoding

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

Let's add another test to push  an even number of tests over the edge from Android batch 62 batch 7.

Comment 15

[Henri Sivonen (:hsivonen)]

Thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/58c6f797275a
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c844a426068

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/58c6f797275a
https://hg.mozilla.org/mozilla-central/rev/0c844a426068