1705363 - Fixes to unified intl NumberFormatterSkeleton implementation

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P1)

Description

[Dan Minor [:dminor]]

With my WIP on Bug 1701695, I can run the SpiderMonkey ecma402 test suite against the unified intl::NumberFormat implementation.

I've found one bug so far: https://searchfox.org/mozilla-central/source/intl/components/src/NumberFormatterSkeleton.cpp#45 should be significantDigits, not fractionDigits.

And Greg noticed that the forward declaration here: https://searchfox.org/mozilla-central/source/intl/components/src/NumberFormatterSkeleton.h#13 is not valid because we use internal definitions of the structure. It should just be a #include "NumberFormat.h".

I still have a handful of test failures in SpiderMonkey, so there may be more problems :/. I'll put a patch up for review here once I work through the remaining failures.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1695937

Comment 2

[Dan Minor [:dminor]]

Try run: https://treeherder.mozilla.org/jobs?repo=try&revision=a024c139f40f900a1757d91ca8e0e87eb739e1dc

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1705363 - Fixes to intl::NumberFormatterSkeleton; r=gregtatum!

These are fixes for issues noticed with NumberFormatterSkeleton while testing
this code in SpiderMonkey against the full set of intl tests there. I've added
minimal test cases for the issues encountered as part of this patch.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ea44f437769
Fixes to intl::NumberFormatterSkeleton; r=gregtatum

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/9ea44f437769

Comment 6

[Dan Minor [:dminor]]

Comment on attachment 9216168 [details]
Bug 1705363 - Fixes to intl::NumberFormatterSkeleton; r=gregtatum!

Beta/Release Uplift Approval Request

• User impact if declined: Potentially incorrect results when rendering numbers through Fluent. As far as I know, there were no user bug reports for this, so it's possible that the affected code paths are not currently used in Fluent.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch is quite small and I checked it against the SpiderMonkey internationalization test suite while working on Bug 1701695 before splitting it out into a separate bug.
• String changes made/needed: None

Comment 7

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9216168 [details]
Bug 1705363 - Fixes to intl::NumberFormatterSkeleton; r=gregtatum!

Low risk, has tests and we are still early in the beta cycle, approved for 89 Beta 3, thanks.

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/c97b27c8173b