-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[encoding] Add tests for GBK and gb18030 encoding #26385
[encoding] Add tests for GBK and gb18030 encoding #26385
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I added another file since inexorabletash's approval. It includes 82 code points that are decoded with GBK differently in Chromium and WebKit before my change. |
Thanks for writing these. There was also #20361 but the i18n WG never got around to finishing those and cleaning them up seemed like quite a bit of effort. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sticking with "Approve" but a last minute observation that many variables aren't declared/scoped (with var
, let
or const
) in many of these files/functions. Probably copy/paste from existing tests so not a big deal to fix, but I wouldn't object if they got a sprinkle of keywords.
These tests come from https://bugs.webkit.org/show_bug.cgi?id=218380 All tests pass in Firefox and in WebKit after that change. Chrome has the following deviations from the standard: 1. The URL GBK encoding of the Yen sign (U+00A5) is %A3%A4 instead of %26%23165%3B 2. Decoding 0x80 with gb18030 results in a replacement character instead of the Euro sign (U+20A0) 3. The GBK decoder does not use the gb18030 decoder, which makes it less permissive.
…it/Chromium and spec.
da9f6ab
to
ec62bb8
Compare
I have rebased this since a bot seemed to be stuck and I also added a commit to clean up the const/let/semicolon stuff since I think I was responsible for the original bad style here. |
Refs: web-platform-tests/wpt#26385 PR-URL: #36659 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Refs: web-platform-tests/wpt#26385 PR-URL: #36659 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>
These tests come from https://bugs.webkit.org/show_bug.cgi?id=218380
All tests pass in Firefox and in WebKit after that change.
Chrome has the following deviations from the standard: