Closed
Bug 1483545
Opened 6 years ago
Closed 6 years ago
Intl.RelativeTimeFormat should treat -0 as indicating a past time
Categories
(Core :: JavaScript: Internationalization API, enhancement)
Core
JavaScript: Internationalization API
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(1 file, 1 obsolete file)
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•6 years ago
|
||
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•6 years ago
|
||
(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.
Assignee | ||
Comment 3•6 years ago
|
||
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.
Assignee | ||
Comment 4•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Comment 5•6 years ago
|
||
Did you run a test which uses "quarter"? That's currently not supported because of bug 1473588.
Comment 6•6 years ago
|
||
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.)
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 7•6 years ago
|
||
OK, thanks. I don't have time to work on this immediately, but I should get to it Monday or so.
Assignee | ||
Comment 8•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Attachment #9017911 -
Attachment is obsolete: true
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 9•6 years ago
|
||
André, the patch in this bug is r?you. No rush, just want to make sure you see it.
Flags: needinfo?(andrebargull)
Comment 10•6 years ago
|
||
I'm stealing the r? to unblock us here. Hope it's ok since I wrote the original patch ;)
Flags: needinfo?(andrebargull)
Updated•6 years ago
|
Attachment #9023120 -
Attachment description: Bug 1483545 - Intl.RelativeTimeFormat: Treat -0 as indicating a past time. r?anba → Bug 1483545 - Intl.RelativeTimeFormat: Treat -0 as indicating a past time. r?gandalf
Comment 11•6 years ago
|
||
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d3b4650e49e1
Intl.RelativeTimeFormat: Treat -0 as indicating a past time. r=zbraniecki
Comment 12•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Updated•6 years ago
|
Keywords: dev-doc-needed
Comment 13•6 years ago
|
||
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.
Keywords: dev-doc-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•