1719550 - Unify Intl APIs in intl/locale/nsCollation.h

Status: RESOLVED FIXED

(Core :: Internationalization, task, P3)

Description

[Greg Tatum [:gregtatum]]

Work: Medium
What it is: An abstraction over the collator

https://searchfox.org/mozilla-central/source/intl/locale/nsCollation.h

Comment 1

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Build an initial unified mozilla::intl::Collator; r?#platform-i18n-reviewers!

This collator has all the features needed for backing nsCollation.h. The tests
focus on making sure the component is wired correctly, but does not worry over
much about correctness of the backing implementation. I felt that this was covered
well by existing tests such as intl/locale/tests/gtest/TestCollation.cpp

Comment 2

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Use the mozilla::intl API for nsCollation; r?#platform-i18n-reviewers

As part of the mozilla::intl unification project, this removes the direct calls
to ICU4C, and instead uses mozilla::intl::Collator.

I included a little bit of simplification by removing the mHasCollator, and
relied on the mCollator to be nullptr instead.

I also removed nsCollation::ConvertStrength so that I could only forward
declare the mozilla::intl::Collator and simplify the header imports. The
function where it was being used ended up being simplified anyway through
the new API.

Depends on D120494

Comment 3

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Unify collation in nsNavHistory.cpp; r?#platform-i18n-reviewers!

This is an experiment to completely remove nsCollation.

Depends on D120495

Comment 4

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Add support for BCP 47 extensions to mozilla::intl::Collator; r?#platform-i18n-reviewers!

SpiderMonkey requires the BCP 47 locale extensions, which involves iterating
over the keywords in ICU, and mapping them to the BCP 47 version. Specifically,
this will change the "phonebook" keyword to "phonebk". This should hopefully
expose a simpler API to SpiderMonkey, the only consumer.

Depends on D120494

Comment 5

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Use mozilla::intl::Collator in the SpiderMonkey; r?#platform-i18n-reviewers!,anba,tcampbell

Depends on D120903

Comment 6

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Add a TryCreateComponent to LocaleService; r?#platform-i18n-reviewers!

This method will allow mozilla::intl components to be initialized in the current
Gecko app's locale.

Depends on D120904

Comment 7

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Unify collator in nsXULSortService and nsXULContentUtils; r?#platform-i18n-reviewers!

Depends on D120526

Comment 8

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Unify collator in txXPathResultComparator; r?#platform-i18n-reviewers!

Depends on D121429

Comment 9

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Unify collator in nsDirectoryIndexStream.cpp; r?#platform-i18n-reviewers!

Depends on D121430

Comment 10

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Unify collator in mozStorageService and SQLCollations; r?#platform-i18n-reviewers!

Depends on D121431

Comment 11

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Remove unused nsLocaleConstructors.h; r?#platform-i18n-reviewers!

Depends on D121432

Comment 12

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Use Intl.Collator in test_locale_collation.js; r?#platform-i18n-reviewers!

This now uses JavaScript's standardized Intl.Collator.

Depends on D121433

Comment 13

[Greg Tatum [:gregtatum]]

Attachment: Bug 1719550 - Remove nsCollation; r?#platform-i18n-reviewers!

All the call sites now use mozilla::intl::Collator

Depends on D121434

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b04162618a1
Build an initial unified mozilla::intl::Collator; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/7b46b5cc7424
Add support for BCP 47 extensions to mozilla::intl::Collator; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/b14702168fb9
Use mozilla::intl::Collator in the SpiderMonkey; r=anba,platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/a8b778ca2375
Add a TryCreateComponent to LocaleService; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/bd125cc5b9b4
Unify collation in nsNavHistory.cpp; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/48f39b5ef77f
Unify collator in nsXULSortService and nsXULContentUtils; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/389a77da6ddc
Unify collator in txXPathResultComparator; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/3ef5792020bb
Unify collator in nsDirectoryIndexStream.cpp; r=platform-i18n-reviewers,necko-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/b4b0d58778e9
Unify collator in mozStorageService and SQLCollations; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/105676dd2d6f
Remove unused nsLocaleConstructors.h; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/5e2019335244
Use Intl.Collator in test_locale_collation.js; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/3b2735a29138
Remove nsCollation; r=platform-i18n-reviewers,nordzilla

Comment 15

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on TestCollator.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/495f05c1de4a5fe9f557c23c168b656116567fec
Log link: https://treeherder.mozilla.org/logviewer?job_id=347971665&repo=autoland&lineNumber=39743

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f05d70fc350c
Build an initial unified mozilla::intl::Collator; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/4c972c58e835
Add support for BCP 47 extensions to mozilla::intl::Collator; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/45228aa42bc7
Use mozilla::intl::Collator in the SpiderMonkey; r=anba,platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/6aad2834ea2f
Add a TryCreateComponent to LocaleService; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/ed5309e3101a
Unify collation in nsNavHistory.cpp; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/07628cc8c768
Unify collator in nsXULSortService and nsXULContentUtils; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/bfa3c9c9e5d3
Unify collator in txXPathResultComparator; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/5e7ff35e0560
Unify collator in nsDirectoryIndexStream.cpp; r=platform-i18n-reviewers,necko-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/9f822c3f4f6a
Unify collator in mozStorageService and SQLCollations; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/184d7eadd3a1
Remove unused nsLocaleConstructors.h; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/fae99cc97f31
Use Intl.Collator in test_locale_collation.js; r=platform-i18n-reviewers,nordzilla
https://hg.mozilla.org/integration/autoland/rev/1a74809df6b9
Remove nsCollation; r=platform-i18n-reviewers,nordzilla

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f05d70fc350c
https://hg.mozilla.org/mozilla-central/rev/4c972c58e835
https://hg.mozilla.org/mozilla-central/rev/45228aa42bc7
https://hg.mozilla.org/mozilla-central/rev/6aad2834ea2f
https://hg.mozilla.org/mozilla-central/rev/ed5309e3101a
https://hg.mozilla.org/mozilla-central/rev/07628cc8c768
https://hg.mozilla.org/mozilla-central/rev/bfa3c9c9e5d3
https://hg.mozilla.org/mozilla-central/rev/5e7ff35e0560
https://hg.mozilla.org/mozilla-central/rev/9f822c3f4f6a
https://hg.mozilla.org/mozilla-central/rev/184d7eadd3a1
https://hg.mozilla.org/mozilla-central/rev/fae99cc97f31
https://hg.mozilla.org/mozilla-central/rev/1a74809df6b9

Comment 18

[Andra Esanu (needinfo me)]

== Change summary for alert #30871 (as of Tue, 10 Aug 2021 16:35:26 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
19%	decision		gecko-decision		144.49 -> 116.75

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30871