1483545 - Intl.RelativeTimeFormat should treat -0 as indicating a past time

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, enhancement)

Description

[Jason Orendorff [:jorendorff]]

Given

    const rtf = new Intl.RelativeTimeFormat("en-US");

the spec requires output like this:

   rtf.format(0, "second") === "in 0 seconds"
   rtf.format(-0, "second") === "0 seconds ago"

I'm disabling a few test262 tests due to this bug.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

I think that's the bug we discovered when we were working on this: https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/js/src/builtin/intl/RelativeTimeFormat.cpp#276-279

Seems like that bug was fixed so maybe just removing the if will fix it?

Comment 2

[André Bargull [:anba]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #1)
> Seems like that bug was fixed so maybe just removing the if will fix it?

Yes, that if-statement needs to be removed. And additionally js/src/tests/non262/Intl/RelativeTimeFormat/format.js needs to be updated to match the new expected behaviour and probably we also want to bump the minimum system ICU version in build/autoconf/icu.m4 to ICU 62.1 to avoid issues when an older system ICU is run against our tests.

Comment 3

[Jason Orendorff [:jorendorff]]

This did not fix it for me. After deleting the if-statement, I get "Error: internal error while computing Intl data" here:

        CallICU(cx, [rtf, t, relDateTimeUnit, relDateTimeNumeric](UChar* chars, int32_t size,
                                                                  UErrorCode* status)
        {
            auto fmt = relDateTimeNumeric == RelativeTimeNumeric::Auto
                       ? ureldatefmt_format
                       : ureldatefmt_formatNumeric;
            return fmt(rtf, t, relDateTimeUnit, chars, size, status);
        });

https://searchfox.org/mozilla-central/rev/eef79962ba73f7759fd74da658f6e5ceae0fc730/js/src/builtin/intl/RelativeTimeFormat.cpp#365-372

The version of ICU in my tree is 62.1. I did a distclean build to make sure.

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: and bug 1483547 - Non-working patch

Comment 5

[André Bargull [:anba]]

Did you run a test which uses "quarter"? That's currently not supported because of bug 1473588.

Comment 6

[André Bargull [:anba]]

We've updated to ICU 63 (bug 1499026, bug 1473588), so we can now apply this patch. (I've tested it locally and didn't hit any errors in non262/Intl nor in test262/intl402.)

Comment 7

[Jason Orendorff [:jorendorff]]

OK, thanks. I don't have time to work on this immediately, but I should get to it Monday or so.

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1483545 - Intl.RelativeTimeFormat: Treat -0 as indicating a past time. r?gandalf

Comment 9

[Jason Orendorff [:jorendorff]]

André, the patch in this bug is r?you. No rush, just want to make sure you see it.

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'm stealing the r? to unblock us here. Hope it's ok since I wrote the original patch ;)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3b4650e49e1
Intl.RelativeTimeFormat: Treat -0 as indicating a past time. r=zbraniecki

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/d3b4650e49e1

Comment 13

[Chris Mills [:cmills]]

Removing the dev-doc-needed flag again. On further inspection, this is more of a bug than a feature, and the whole thing was only implemented in Fx65, so I don't think it's really worth reporting this separately.