Bug 223103 - Derive index EUC-KR from ICU
Summary: Derive index EUC-KR from ICU
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-11 16:38 PST by Alex Christensen
Modified: 2021-03-17 11:29 PDT (History)
2 users (show)

See Also:


Attachments
Patch (4.01 KB, patch)
2021-03-11 16:40 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (4.37 KB, patch)
2021-03-11 21:40 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (4.41 KB, patch)
2021-03-12 07:59 PST, Alex Christensen
ysuzuki: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-03-11 16:38:37 PST
Derive index EUC-KR from ICU
Comment 1 Alex Christensen 2021-03-11 16:40:12 PST
Created attachment 422991 [details]
Patch
Comment 2 Alex Christensen 2021-03-11 21:40:52 PST
Created attachment 423014 [details]
Patch
Comment 3 Alex Christensen 2021-03-12 07:59:06 PST
Created attachment 423049 [details]
Patch
Comment 4 Yusuke Suzuki 2021-03-12 16:22:34 PST
Comment on attachment 423049 [details]
Patch

r=me
Comment 5 Darin Adler 2021-03-12 17:22:07 PST
Comment on attachment 423049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423049&action=review

> Source/WebCore/platform/text/EncodingTables.cpp:8654
> -        ASSERT(isSortedByFirst(eucKRDecodingIndex));
> -        ASSERT(sortedFirstsAreUnique(eucKRDecodingIndex));
> +        ASSERT(isSortedByFirst(eucKR()));
> +        ASSERT(sortedFirstsAreUnique(eucKR()));

Wouldn’t you want to keep both of these?
Comment 6 Alex Christensen 2021-03-12 20:29:29 PST
Comment on attachment 423049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423049&action=review

>> Source/WebCore/platform/text/EncodingTables.cpp:8654
>> +        ASSERT(sortedFirstsAreUnique(eucKR()));
> 
> Wouldn’t you want to keep both of these?

I don't t think I understand this comment.  I am keeping both of these
Comment 7 EWS 2021-03-12 20:57:53 PST
commit-queue failed to commit attachment 423049 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 8 EWS 2021-03-12 22:09:26 PST
commit-queue failed to commit attachment 423049 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 9 Darin Adler 2021-03-14 13:20:08 PDT
Comment on attachment 423049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423049&action=review

>>> Source/WebCore/platform/text/EncodingTables.cpp:8654
>>> +        ASSERT(sortedFirstsAreUnique(eucKR()));
>> 
>> Wouldn’t you want to keep both of these?
> 
> I don't t think I understand this comment.  I am keeping both of these

I was suggesting we could separately assert both that eucKRDecodingIndexReference is sorted and unique and that the actual generated table is?
Comment 10 Alex Christensen 2021-03-17 10:36:42 PDT
We assert that they are equal when generating the table.
Comment 11 Alex Christensen 2021-03-17 10:38:04 PDT
r274569
Comment 12 Darin Adler 2021-03-17 11:29:44 PDT
(In reply to Alex Christensen from comment #10)
> We assert that they are equal when generating the table.

Oh, right, that should have been obvious to me!