Skip to content

Commit

Permalink
Bug 1855749 - Part 3: Correctly handle ignorePunctuation option. r=dm…
Browse files Browse the repository at this point in the history
…inor

The default value for `GetOption` in `InitializeCollator` is changed from `false`
to `undefined` to avoid having to `intl_isIgnorePunctuation` in the constructor
function.

The corresponding spec PR is <tc39/ecma402#833>, but
our behaviour is already not strictly spec-compliant as described in
<tc39/ecma402#832>, so it seems reasonable to simply
implement the spec PR ahead of time. (We don't want to "fix" our implementation
to strictly follow the spec without the PR, because that could be web-incompatible
resp. at least disruptive for Thai users, which currently already get the expected
behaviour where punctuation characters are ignored in Thai.)

Differential Revision: https://phabricator.services.mozilla.com/D189545

UltraBlame original commit: bcf3b4652f995f4f52792d5897cd2d905d5a029f
  • Loading branch information
marco-c committed Nov 8, 2023
1 parent 992a666 commit 3e94b30
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 6 deletions.
2 changes: 1 addition & 1 deletion intl/components/src/Collator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ ICUResult Collator::SetOptions(const Options& aOptions,

result = this->SetAlternateHandling(
aOptions.ignorePunctuation ? Collator::AlternateHandling::Shifted
: Collator::AlternateHandling::Default);
: Collator::AlternateHandling::NonIgnorable);
if (result.isErr()) {
return result;
}
Expand Down
9 changes: 7 additions & 2 deletions js/src/builtin/intl/Collator.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ function resolveCollatorInternals(lazyCollatorData) {
internalProps.sensitivity = s;


internalProps.ignorePunctuation = lazyCollatorData.ignorePunctuation;
var ignorePunctuation = lazyCollatorData.ignorePunctuation;
if (ignorePunctuation === undefined) {
var actualLocale = collatorActualLocale(r.dataLocale);
ignorePunctuation = intl_isIgnorePunctuation(actualLocale);
}
internalProps.ignorePunctuation = ignorePunctuation;



Expand Down Expand Up @@ -231,7 +236,7 @@ function InitializeCollator(collator, locales, options) {
lazyCollatorData.rawSensitivity = s;


var ip = GetOption(options, "ignorePunctuation", "boolean", undefined, false);
var ip = GetOption(options, "ignorePunctuation", "boolean", undefined, undefined);
lazyCollatorData.ignorePunctuation = ip;


Expand Down
3 changes: 0 additions & 3 deletions js/src/tests/jstests.list
Original file line number Diff line number Diff line change
Expand Up @@ -612,9 +612,6 @@ skip script test262/built-ins/AsyncGeneratorPrototype/return/return-suspendedYie
# Requires Unicode 15.1
skip script test262/built-ins/RegExp/unicodeSets/generated/rgi-emoji-15.1.js

# Default value for ignorePunctuation incorrectly computed.
skip script test262/intl402/Collator/prototype/compare/ignorePunctuation.js

# Time zone canonicalization proposal.
skip script test262/intl402/DateTimeFormat/timezone-not-canonicalized.js
skip script test262/intl402/DateTimeFormat/timezone-case-insensitive.js
Expand Down
60 changes: 60 additions & 0 deletions js/src/tests/non262/Intl/Collator/ignorePunctuation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@


function testPunctuation(col, expectedIgnorePunctuation) {
let ignorePunctuation = col.resolvedOptions().ignorePunctuation;
assertEq(ignorePunctuation, expectedIgnorePunctuation);


assertEq(col.compare("", "*"), ignorePunctuation ? 0 : -1);


assertEq(col.compare("", " "), ignorePunctuation ? 0 : -1);
}

const locales = [
"en", "de", "fr", "it", "es", "ar", "zh", "ja",


"th", "th-Thai", "th-TH", "th-u-kf-false",
];

for (let locale of locales) {

let isThai = new Intl.Locale(locale).language === "th";


testPunctuation(new Intl.Collator(locale, {}), isThai);


for (let ignorePunctuation of [true, false]) {
testPunctuation(new Intl.Collator(locale, {ignorePunctuation}), ignorePunctuation);
}
}

if (typeof getDefaultLocale === "undefined") {
var getDefaultLocale = SpecialPowers.Cu.getJSTestingFunctions().getDefaultLocale;
}
if (typeof setDefaultLocale === "undefined") {
var setDefaultLocale = SpecialPowers.Cu.getJSTestingFunctions().setDefaultLocale;
}

const defaultLocale = getDefaultLocale();

function withLocale(locale, fn) {
setDefaultLocale(locale);
try {
fn();
} finally {
setDefaultLocale(defaultLocale);
}
}


for (let locale of ["th", "th-TH"]) {
withLocale(locale, () => {
testPunctuation(new Intl.Collator(undefined, {}), true);
});
}

if (typeof reportCompare === "function")
reportCompare(true, true);

0 comments on commit 3e94b30

Please sign in to comment.