Use case mapping APIs from ICU instead of building and maintaining our own mapping table

RESOLVED FIXED in Firefox 52



3 years ago
3 years ago


(Reporter: jfkthame, Assigned: jfkthame)


(Blocks 1 bug)


Firefox Tracking Flags

(firefox52 fixed)



(4 attachments)



3 years ago
When ICU is available in the build, there's no need for us to have our own tables of upper/lower/titlecase mappings to support the case-conversion methods in intl/unicharutil/util. We can simply call ICU's case mapping functions. This should save around 14K of data, AFAICS.

Comment 1

3 years ago
We've done this for a bunch of other properties already, but never got around to case mappings. This updates the generation tool so that the case mappings will be excluded from builds where ICU is available, saving about 14K of data size.
Attachment #8795290 - Flags: review?(m_kato)


3 years ago
Assignee: nobody → jfkthame

Comment 2

3 years ago
And this patch (to be merged with pt 1 for landing) makes the mozilla::unicode::Get*case functions into simple wrappers around the ICU functions.
Attachment #8795292 - Flags: review?(m_kato)

Comment 3

3 years ago
While we're here, I think it will make the code a lot easier to read if we collect all the ICU-based wrappers together in one #if-block, instead of scattering #if-#else-#endif through each individual accessor. This will help us to see how much code is really left here, and how much can be dropped entirely once we no longer need to support an ICU-free build.
Attachment #8795293 - Flags: review?(m_kato)

Comment 4

3 years ago
Looks like we also need to do this, to avoid build bustage from duplicate macro definitions; now that we're pulling in ICU headers more widely, we don't need these copies in nsBidi.h. (This will also need to be folded together with the previous patch to avoid the risk of broken bisections.)
Attachment #8795303 - Flags: review?(m_kato)
Attachment #8795290 - Flags: review?(m_kato) → review+
Attachment #8795303 - Flags: review?(m_kato) → review+
Attachment #8795292 - Flags: review?(m_kato) → review+
Comment on attachment 8795293 [details] [diff] [review]
pt 3 - Clean up/simplify use of ENABLE_INTL_API conditionals in nsUnicodeProperties (code rearrangement, no change in behavior)

Review of attachment 8795293 [details] [diff] [review]:

::: intl/unicharutil/util/nsUnicodeProperties.h
@@ +95,5 @@
> +{
> +  UNumericType type =
> +    UNumericType(u_getIntPropertyValue(aCh, UCHAR_NUMERIC_TYPE));
> +  return type == U_NT_DECIMAL || type == U_NT_DIGIT
> +         ? int8_t(u_getNumericValue(aCh)) : -1;  

nits: Trim space after -1;.
Attachment #8795293 - Flags: review?(m_kato) → review+

Comment 7

3 years ago
Bug 1305700 - pt 1 & 2 - Exclude case mappings from nsUnicodePropertyData.cpp, and use ICU case mappings instead of our own table when building with ENABLE_INTL_API. r=m_kato
Bug 1305700 - pt 3 & 4 - Clean up/simplify use of ENABLE_INTL_API conditionals in nsUnicodeProperties (code rearrangement, no change in behavior). r=m_kato

Comment 8

3 years ago
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.