Skip to content

Correct GB18030-2022 encode behavior#33769

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
annevk:eng/Correct-GB18030-2022-encode-behavior
Sep 18, 2024
Merged

Correct GB18030-2022 encode behavior#33769
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
annevk:eng/Correct-GB18030-2022-encode-behavior

Conversation

@annevk
Copy link
Contributor

@annevk annevk commented Sep 17, 2024

068d177

Correct GB18030-2022 encode behavior
https://bugs.webkit.org/show_bug.cgi?id=279822

Reviewed by Alex Christensen.

This does two things:

1. It restores asymmetric encode as it was in 265633@main to not emit
   two additional leading 0x00 bytes.
2. It removes the four-byte decode table as it duplicates the
   information already in gb18030RangesCodePoint(). No need to store
   it twice.

* LayoutTests/imported/w3c/web-platform-tests/encoding/legacy-mb-schinese/gb18030/gb18030-encoder.html:
* Source/WebCore/PAL/pal/text/TextCodecCJK.cpp:
(PAL::gb18030AsymmetricEncode):
(PAL::TextCodecCJK::gb18030Decode):
(PAL::gbEncodeShared):
(PAL::gb180302022Encode): Deleted.
(PAL::gb180302022Decode): Deleted.

Canonical link: https://commits.webkit.org/283826@main

df5b778

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 wincairo-tests
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🧪 api-wpe
🧪 ios-wk2-wpt 🧪 mac-wk1 ✅ 🛠 wpe-cairo
🧪 api-ios 🧪 mac-wk2 ✅ 🛠 gtk
loading 🛠 🧪 jsc-arm64 ✅ 🛠 vision 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 vision-sim 🧪 mac-wk2-stress 🧪 api-gtk
🧪 vision-wk2 🧪 mac-intel-wk2
✅ 🛠 🧪 unsafe-merge 🛠 tv
🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@annevk annevk self-assigned this Sep 17, 2024
@annevk annevk added the Text For bugs in text layout and rendering, including international text support. label Sep 17, 2024
@vitorroriz
Copy link
Contributor

vitorroriz commented Sep 17, 2024

This does two things:

  1. It restores asymmetric encode as it was in 265633@main.
  2. It removes the four-byte decode table as it duplicates the
    information already in gb18030RangesCodePoint(). No need to store
    it twice.

Can you add to the description why we want to do [1]? Is this about the renaming?

@annevk
Copy link
Contributor Author

annevk commented Sep 17, 2024

It's the renaming and but more importantly doing it correctly, i.e., not emitting zero bytes. I guess I can call that out in the final commit message, but I'll let the bots run for now.

@annevk annevk force-pushed the eng/Correct-GB18030-2022-encode-behavior branch from 7341259 to df5b778 Compare September 18, 2024 06:25
@annevk annevk added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 18, 2024
https://bugs.webkit.org/show_bug.cgi?id=279822

Reviewed by Alex Christensen.

This does two things:

1. It restores asymmetric encode as it was in 265633@main to not emit
   two additional leading 0x00 bytes.
2. It removes the four-byte decode table as it duplicates the
   information already in gb18030RangesCodePoint(). No need to store
   it twice.

* LayoutTests/imported/w3c/web-platform-tests/encoding/legacy-mb-schinese/gb18030/gb18030-encoder.html:
* Source/WebCore/PAL/pal/text/TextCodecCJK.cpp:
(PAL::gb18030AsymmetricEncode):
(PAL::TextCodecCJK::gb18030Decode):
(PAL::gbEncodeShared):
(PAL::gb180302022Encode): Deleted.
(PAL::gb180302022Decode): Deleted.

Canonical link: https://commits.webkit.org/283826@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Correct-GB18030-2022-encode-behavior branch from df5b778 to 068d177 Compare September 18, 2024 06:27
@webkit-commit-queue
Copy link
Collaborator

Committed 283826@main (068d177): https://commits.webkit.org/283826@main

Reviewed commits have been landed. Closing PR #33769 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 068d177 into WebKit:main Sep 18, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 18, 2024
@annevk annevk deleted the eng/Correct-GB18030-2022-encode-behavior branch September 18, 2024 06:41
aarongable pushed a commit to chromium/chromium that referenced this pull request Sep 26, 2024
This CL imports the following changes in WebKit, and modified the code
to the Chromium coding style.
- WebKit/WebKit#33699
- WebKit/WebKit#33769
- WebKit/WebKit#33836

Additional change:
- resolved TODO, which I accidentally left in the last commit.

Bug: 367985624
Change-Id: Ifa4517e972ba025b3ea0bf7cfcf29e11a101c5ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5890346
Auto-Submit: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1360339}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Text For bugs in text layout and rendering, including international text support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments