Closed Bug 959058 Opened 6 years ago Closed 6 years ago

Make gbk use the gb18030 decoder

Categories

(Core :: Internationalization, defect, P4, minor)

defect

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: hsivonen, Assigned: hsivonen)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

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.
See Also: → 951691
Blocks: 959061
(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...
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?
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Flags: needinfo?(VYV03354)
I have no strong opinion about that. Feel free to choose whichever you like.
Flags: needinfo?(VYV03354)
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.
Assignee: hsivonen → nobody
Status: ASSIGNED → NEW
Priority: -- → P4
(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. :-(
(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...
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.
(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.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
I'll file another bug about removing dead code.
Attachment #8359101 - Attachment is obsolete: true
Attachment #8365508 - Flags: review?(VYV03354)
Attachment #8365508 - Flags: review?(VYV03354) → review+
Blocks: 964225
Caused by bug 962029.
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.
Attachment #8365913 - Flags: review?(VYV03354)
Attachment #8365913 - Flags: review?(VYV03354) → review+
https://hg.mozilla.org/mozilla-central/rev/58c6f797275a
https://hg.mozilla.org/mozilla-central/rev/0c844a426068
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.