1407240 - Add mozIntl.RelativeTimeFormat

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

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

With bug 1270140 landed, we now want to expose the API to Mozilla engineers via mozIntl.

I think it makes sense to get a bit more sophisticated on this level and actually add support to some bestFit algorithm that will resolve to the best unit.

Comment 1

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

Attachment: Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Review commit: https://reviewboard.mozilla.org/r/188098/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/188098/

Comment 2

[Jonathan Kew [:jfkthame]]

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

https://reviewboard.mozilla.org/r/188098/#review193534

This looks generally nice, but I have a question about how rounding is done -- is this a deliberate choice (does it follow expectations of some spec, or other browsers behavior, or...?) or should we reconsider it?

::: toolkit/components/mozintl/mozIntl.js:44
(Diff revision 1)
> +const DAYS_PER_WEEK = 7;
> +const HOURS_PER_DAY = 24;
> +const MINUTES_PER_HOUR = 60;
> +const SECONDS_PER_MINUTE = 60;
> +const MS_PER_SECOND = 1000;
> +const MS_PER_MINUTE = 60000;
> +const MS_PER_HOUR = 3600000;
> +const MS_PER_DAY = 86400000;
> +const MS_PER_WEEK = 604800000;
> +const MS_PER_400_YEARS = 88359465600000;

I don't see anything that uses these; are they actually needed?

If we do need them (e.g. for a possible revision of computeTimeUnits below), could we express them as understandable expressions rather than such large magic numbers? E.g. MS_PER_DAY = MS_PER_HOUR * 24, and so on.

::: toolkit/components/mozintl/mozIntl.js:61
(Diff revision 1)
> +  const millisecond = Math.round(v);
> +  const second = Math.round(millisecond / 1000);
> +  const minute = Math.round(second / 60);
> +  const hour = Math.round(minute / 60);
> +  const day = Math.round(hour / 24);

I wonder if we really want to do the successive rounding here, or if each of these should be computed and rounded independently from the original value? It will make a difference in the edge cases as `v` approaches (but doesn't quite reach) the half-way point of one of the larger units, because rounding-up at a smaller unit will then reach the half-way point of the larger one, leading to rounding-up there as well.

::: toolkit/components/mozintl/mozIntl.js:81
(Diff revision 1)
> +    year: Math.round(rawYear)
> +  };
> +}
> +
> +function getBestMatchUnit(units) {
> +  if (Math.abs(units.second) < 45) { return 'second'; }

nit, please line-break consistently with the following statements

Comment 3

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

(In reply to Jonathan Kew (:jfkthame) from comment #2)
> Comment on attachment 8917090 [details]
> Bug 1407240 - Add mozIntl.RelativeTimeFormat.
> 
> https://reviewboard.mozilla.org/r/188098/#review193534
> 
> This looks generally nice, but I have a question about how rounding is done
> -- is this a deliberate choice (does it follow expectations of some spec, or
> other browsers behavior, or...?) or should we reconsider it?

I took it from the original proposal for Intl.RelativeTime which planned to add `bestFit` unit - you can find it here: https://github.com/tc39/proposal-intl-relative-time/blob/0bb04ff6bb6d1b922d620de01e15ab358b545f44/spec/relativetimeformat.html#L207

I then implemented it for FirefoxOS - https://github.com/mozilla-b2g/gaia/blob/master/shared/js/moz_intl.js#L279


ECMA402 since then decided to postpone this feature until version 2.0 of the API to simplify it, and expects higher level libraries to implement their own unit selection, which is what I'd like to do here.

I'm not very strongly opinionated on the algorithm we use, nor on exact rounding.
 
> I wonder if we really want to do the successive rounding here, or if each of
> these should be computed and rounded independently from the original value?
> It will make a difference in the edge cases as `v` approaches (but doesn't
> quite reach) the half-way point of one of the larger units, because
> rounding-up at a smaller unit will then reach the half-way point of the
> larger one, leading to rounding-up there as well.

Do you have a preference?


Also, as far as I understand, there are three distinct approaches to unit selection:

 * humanized - where, for example, anything above 45 minutes is expressed as "an hour"
 * medium - where anything above 55 minutes is expressed as "an hour"
 * strict - where only 60 minutes and up are expressed as "an hour"

For the humanized, we can also follow what momentjs does: https://momentjs.com/docs/#/displaying/fromnow/

My plan was to start with the humanized form, and if enough code needs more strict forms, we can add them.

Pike, flod, jfkthame - any preferences here?

Comment 4

[Axel Hecht [:Pike]]

I wonder if there are differences between cultures in what an hour is.

Also, I think that that's a question much for UX than for l10n.

Comment 5

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

Philipp, back in January I asked you for feedback on what capabilities you'd like to see - this bug is bringing one of them to Firefox devs :)

Can you provide feedback on what would you like to see in terms of rounding strategies (comment 3)?

Comment 6

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

> I wonder if there are differences between cultures in what an hour is.

There may be, but I don't think we should aim to solve it in the first iteration. The aim of this API is to replace DIY relative time code snippets all around our codebase, and none of them is culturally sensitive. Also, momentjs isn't either.
So I'm happy to get the first step landed, which already positions us better for a conversation like that.

Comment 7

[Francesco Lodolo [:flod]]

59' is not an hour, but that's me being more German than Germans :)

Given the context (software), I would prefer using strict than medium as a start (I shiver at the thought of a calendar app using the humanized algorithm).

Having said that, I think it's indeed a good question for UX.

Comment 8

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Zibi and I talked on Slack about this the other day. I'm still not sure if I fully understand the scope, but here's my take:

I think at least for the Firefox front-end and related pages (like Accounts), the humanized form is all we'll ever need (or at least the 95% case).
I'd also argue that this form would work best for most applications (since most applications are, for the most part, used by humans ;)
AIUI, this API would still allow the developer to explicitly request a precise relative time (or just use absolute time), so the applications that need it, could still be made as precise as they want to be.

Another discussion point was when to start using »yesterday«.
IMO, we should pick some time in the early morning (say, 3am) and if
a) the current time is greater than that time
b) the time to be formatted is on the day before
switch to using »yesterday«.

Hope that helps!

Comment 9

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188098/diff/1-2/

Comment 10

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

I went forward with two strategies for now:

"humanized" follows +/- moment.js (I mainly added `week`).
"strict" is, well, strict.

If we see a need we can fine tune the humanized strategy very easily or/and add another one as the need arises.

> Another discussion point was when to start using »yesterday«.
> IMO, we should pick some time in the early morning (say, 3am) and if
> a) the current time is greater than that time
> b) the time to be formatted is on the day before
> switch to using »yesterday«.

This is unfortunately very tricky. Here's a very detailed analysis of the problem: https://github.com/tc39/proposal-intl-relative-time/issues/14

The tl;dr is - relative "yesterday" cannot be computed from delta. It needs to understand the reference point and timezone of the user. This is something that goes pretty far beyond of what this API can do and is very, very easy to get wrong.

I believe we should leave it for more sophisticated libs like moment.js to handle such scenarios or we can play with the ideas to handle that in a later revision.
For now, I'd like to offload all the custom DIY solutions we have around the code that don't handle relative yesterday anyway.

So, ready for review Jonathan!

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

https://reviewboard.mozilla.org/r/188098/#review202226

::: toolkit/components/mozintl/mozIntl.js:63
(Diff revision 2)
> +  const minute = Math.round(second / 60);
> +  const hour = Math.round(minute / 60);
> +  const day = Math.round(hour / 24);
> +  const rawYear = day * 400 / 146097;

I'm still uneasy about the use of successive roundings here, which will lead to  unexpected results due to an accumulation of error: the larger units will start to be rounded up when the fractional value is rather less than .5, instead of rounding down for all fractions < .5 and up for fractions >= .5, as would normally be expected.

Thus,

    f.formatBestUnit(1.4 * 60 * 60 * 1000)
    "in 1 hour"
    f.formatBestUnit(1.5 * 60 * 60 * 1000)
    "in 2 hours"

looks exactly as expected, but

    f.formatBestUnit(1.49 * 60 * 60 * 1000)
    "in 1 hour"
    f.formatBestUnit(1.491 * 60 * 60 * 1000)
    "in 1 hour"
    f.formatBestUnit(1.492 * 60 * 60 * 1000)
    "in 2 hours"

Surprise!

Similarly,

    f.formatBestUnit(1.47 * 24 * 60 * 60 * 1000)
    "tomorrow"

but

    f.formatBestUnit(1.48 * 24 * 60 * 60 * 1000)
    "in 2 days"

even though 1.48 days is 29 minutes short of being a day and a half.

Comment 12

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188098/diff/2-3/

Comment 13

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

Finally found time to update the patch! Added your edge cases to tests :)

Comment 14

[Jonathan Kew [:jfkthame]]

Thanks for adding these, and fixing the rounding; looks good.

One other thing I wonder about, though (sorry, should have brought this up previously):

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #10)
> This is unfortunately very tricky. Here's a very detailed analysis of the
> problem: https://github.com/tc39/proposal-intl-relative-time/issues/14

Doesn't essentially the same issue apply to "tomorrow", and to other phrases like "last year" or "next year"?

If it's now 1 AM, and I ask for formatBestUnit(22 * 60 * 60 * 1000), which would be 22 hours from now, that works out to 11 PM (today). But formatBestUnit will see that it's nearly a whole day, so it'll pick the "day" unit; and +1 day will result in "tomorrow", which is clearly wrong. :(

Similarly, formatBestUnit(320 * 24 * 60 * 60 * 1000) will return "next year", but if I call this in January, 320 days later may only be mid-November. Or right now, formatBestUnit(-320 * 24 * 60 * 60 * 1000) returns "last year", but the day in question is actually January 11th (of this year). And formatBestUnit(-26 * 24 * 60 * 60 * 1000) says "last month", but 26 days ago is November 1st (clearly "this month").

I'm not sure what I think should be done about this.... ISTM that these phrases are inherently problematic, unless resolved in relation to the current date/time (not only the interval). That would have the effect that a call like formatBestUnit(-26 * 24 * 60 * 60 * 1000) would return different results depending on when it is called, which makes testing more, umm, "interesting"; but wouldn't it be better from a UX point of view? Returning "human-friendly" results that are sometimes downright wrong -- like "last month" for a relative date that in fact resolves to the first of *this* month -- seems like a poor outcome.

Comment 15

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

Thanks for drilling the issue Jonathan!

There a great analysis of the problem scope by rxavier: https://github.com/tc39/proposal-intl-relative-time/issues/14

Rafael also wrote a small library which handles discrete distances: https://github.com/rxaviers/relative-time/

I see two, non exclusive, ways forward:

1) We can extend JS Intl.RelativeTimeFormat to support type `text`|`numeric` as per my proposal in [0] (which roughly translates to ureldat_format vs ureldat_formatNumeric), and in formatBestUnit use the numeric form.

This would make us be able to return "in 1 year" and "1 day ago" instead of "next year" and "yesterday" which I think would work for the examples you're giving.

If it's 1 AM and you ask for time 22 hours from now, then getting "in 1 day" I think is ok, even if it's technically the same day. It's certainly better than getting "tomorrow".

Similarly, getting "in 1 year" for 320 days from now should be ok even if you ask in January about November (10.6 months from now).

I would then work with the ECMA402 to elevate the importance of that option for the first version of the API.

2) We can look into Rafael's RelativeTime library and modify the formatBestUnit to actually take a date (and optionally a reference date instead of using now - useful for testing). This would allow us to get meaningful `text` form.

This would allow us to use a nicer, textual form, but would require us to ask for a date, rather than delta in ms.

I believe that such two-prong attack may work and give us a good short-term solution in form of (1), while we work on an ultimate solution in form of (2). WDYT?

The one issue I'd like to get your help with is designing the custom API extension for the calculated `bestFit` mode. Since we'll want to allow for `formatSomethingSomething` that takes a delta and uses `numeric` form, and another `formatSomethingElse` that takes a date and uses `text` form, I'm wondering how to name them.
Additionally, the `type: numeric|text` would be probably a param on the constructor, so it almost feels like we'll end up with "if you created the RTF with type: numeric, you get this `formatBestUnit`, and if you created the RTF with type: text, you get another and they differ in the type of variable they expect".

My only idea is to always require date as a value and optionally `now` as an anchor. WDYT?


For the reference, here are some examples of DIY solutions for relative time currently in our code:

1) https://searchfox.org/mozilla-central/source/toolkit/mozapps/downloads/DownloadUtils.jsm#353
2) https://searchfox.org/mozilla-central/source/toolkit/content/aboutSupport.js#150
3) https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.js#3321
4) https://searchfox.org/mozilla-central/source/toolkit/components/places/nsNavHistory.cpp#1672

Also, seems like in Java we already do date-related operations:
5) https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/ClientsAdapter.java#370

[0] https://github.com/tc39/proposal-intl-relative-time/issues/9

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #15)
> I see two, non exclusive, ways forward:
> 
> 1) We can extend JS Intl.RelativeTimeFormat to support type `text`|`numeric`
> as per my proposal in [0] (which roughly translates to ureldat_format vs
> ureldat_formatNumeric), and in formatBestUnit use the numeric form.
> 
> This would make us be able to return "in 1 year" and "1 day ago" instead of
> "next year" and "yesterday" which I think would work for the examples you're
> giving.
> 
> If it's 1 AM and you ask for time 22 hours from now, then getting "in 1 day"
> I think is ok, even if it's technically the same day. It's certainly better
> than getting "tomorrow".
> 
> Similarly, getting "in 1 year" for 320 days from now should be ok even if
> you ask in January about November (10.6 months from now).

Yes, IMO "in 1 day" or "in 1 year" for these examples is better than "tomorrow" or "next year". Thinking about it, I guess my interpretation is that "in 1 year" means "a certain period from now, which can reasonably be rounded to 1 year, whether it's actually 11 months or 13 months or...", while "next year" means "in the next calendar year from the current one".

So given that it's currently late November, "next year" starts in little more than a month from now (although in practice I probably wouldn't use the phrase for the very early part of the year, where I might say "in the new year" instead), and runs until about 13 months from now. Whereas "in 1 year" covers a somewhat vague period from late 2018 until mid-2019 or thereabouts.

And so for an API that is designed to format a delta-time in a predictable, current-time-independent way, the "in N <units>" form seems preferable to "human-friendly" terms that identify a specific calendar period relative to "now".

> 2) We can look into Rafael's RelativeTime library and modify the
> formatBestUnit to actually take a date (and optionally a reference date
> instead of using now - useful for testing). This would allow us to get
> meaningful `text` form.
> 
> This would allow us to use a nicer, textual form, but would require us to
> ask for a date, rather than delta in ms.

Or (equivalently) ask for a delta in ms from a reference date, defaulting to "now".

> 
> I believe that such two-prong attack may work and give us a good short-term
> solution in form of (1), while we work on an ultimate solution in form of
> (2). WDYT?

Yes, this sounds reasonable to me.

> 
> The one issue I'd like to get your help with is designing the custom API
> extension for the calculated `bestFit` mode. Since we'll want to allow for
> `formatSomethingSomething` that takes a delta and uses `numeric` form, and
> another `formatSomethingElse` that takes a date and uses `text` form, I'm
> wondering how to name them.
> Additionally, the `type: numeric|text` would be probably a param on the
> constructor, so it almost feels like we'll end up with "if you created the
> RTF with type: numeric, you get this `formatBestUnit`, and if you created
> the RTF with type: text, you get another and they differ in the type of
> variable they expect".
> 
> My only idea is to always require date as a value and optionally `now` as an
> anchor. WDYT?

What about making the "anchor" date also a param to the RelativeTimeFormat constructor? So a text-form RTF will format dates relative to a constant anchor that is determined (defaulting to Now) when it is constructed, not relative to an anchor that is passed on every formatBestUnit call (defaulting to a different Now each time).

(Consider a UI that is listing a bunch of items, all of which have the same timestamp, but formatting each timestamp with formatBestUnit relative to a constantly-changing Now anchor.... there'll be an edge-case risk that the first part of the list appears with "yesterday", and then the next part, processed a couple of ms later, shows "2 days ago". Using a single RTF whose anchor timestamp was fixed at creation time will avoid this inconsistency.)

ISTM that for RelativeTimeFormat purposes, passing the "target" timestamp as a specific Date (rather than a delta in ms) is likely to be more convenient most of the time; but assuming there is an anchor date in the object, the two options are completely equivalent.

In the absence of an anchor, perhaps RelativeTimeFormat isn't really the right terminology, and we should be thinking of something like a TimeDurationFormat for formatting a given period (positive or negative) to an expression like "in 2 hours" or "3 days ago", in a current-time-independent way. That would be the appropriate thing for a UI like "estimated time remaining to download...", whereas RelativeTimeFormat (anchored to the current time) would be appropriate for things like timestamps.

Comment 17

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

> Or (equivalently) ask for a delta in ms from a reference date, defaulting to "now".

That will only work for as long as both anchor and value use the same timezone. I'd prefer to design an API that doesn't limit us here.

> (Consider a UI that is listing a bunch of items, all of which have the same timestamp, but formatting each timestamp with formatBestUnit relative to a constantly-changing Now anchor.... there'll be an edge-case risk that the first part of the list appears with "yesterday", and then the next part, processed a couple of ms later, shows "2 days ago". Using a single RTF whose anchor timestamp was fixed at creation time will avoid this inconsistency.)

At a cost - I'd have to invalidate the formatter for each run.

> In the absence of an anchor, perhaps RelativeTimeFormat isn't really the right terminology, and we should be thinking of something like a TimeDurationFormat for formatting a given period (positive or negative) to an expression like "in 2 hours" or "3 days ago", in a current-time-independent way. That would be the appropriate thing for a UI like "estimated time remaining to download...", whereas RelativeTimeFormat (anchored to the current time) would be appropriate for things like timestamps.

That's a bit tricky. Currently, ICU (and ECMA402) talk about three types of formatters:

 - DurationFormat formats intervals to their numerical form (think: video player UI) - "04:23:15" or "-03:23"
 - RelativeTimeFormat formats relative time like "in 2 days", "yesterday", "1 hour ago"
 - UnitFormat formats absolute dates "2 days", "10 hours", "1 second"

The example of "estimated time remaining to download: 2 minutes" would be best suited by the UnitFormat imho.

For relative time format, until we get the capitalization option [0], it's best to use it for standalone cases like "Last viewed: 10 hours ago".

I'll tinker with it - maybe we could allow for both - Number and Date as an input when used with type: numeric, and only allow for Date when used with type: text?

Comment 18

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188098/diff/3-4/

Comment 19

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

I spent wayyy too much time trying to figure out if there's a way to optimize this code.

There probably is.

I took most of it from https://github.com/rxaviers/relative-time/ and will work with Rafael on updating his library to use Intl.RelativeTimeFormat directly which may result in updates to this code.

But I think it's beyond the point when it's reasonable to delay adding this useful feature for our Firefox UI because I don't seem to be making progress.

The code is semi-simple. It's a single function really, with three helper functions, some thresholds and that's it.

Most importantly - it works. It does take time delta and anchor into account when calculating things and the test coverage seems to prove it.

I'm open to further refinements but I believe I hit the wall and need another look at the code to decide if it needs more work or is it good to land. It's been much longer in baking than it should already :(

Jonathan - can you take a look and decide if this is r? or f? ready?

Comment 20

[Code Review Bot [:reviewbot]]

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

https://reviewboard.mozilla.org/r/188098/#review224622


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/mozintl/mozIntl.js:79
(Diff revision 4)
> +  }
> +  return date;
> +}
> +
> +function bestFit(absDiff) {
> +  switch(true) {

Error: Expected space(s) after "switch". [eslint: keyword-spacing]

::: toolkit/components/mozintl/mozIntl.js:89
(Diff revision 4)
> +    case absDiff.days > 0 && absDiff.hours > threshold.hour: return "day";
> +    case absDiff.hours > 0 && absDiff.minutes > threshold.minute: return "hour";
> +    case absDiff.minutes > 0 && absDiff.seconds > threshold.second: return "minute";
> +    default: return "second";
> +  }
> +};

Error: Unnecessary semicolon. [eslint: no-extra-semi]

::: toolkit/components/mozintl/mozIntl.js:244
(Diff revision 4)
> +          return Math.abs(diff.seconds);
> +        });
> +
> +        let unit = bestFit(absDiff);
> +
> +        switch(unit) {

Error: Expected space(s) after "switch". [eslint: keyword-spacing]

Comment 21

[Jonathan Kew [:jfkthame]]

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

https://reviewboard.mozilla.org/r/188098/#review227408

Somehow the choice of best-fit units and rounding here doesn't always feel quite "right" to me, judging by the expected test results. E.g. if we're going to return "in X minutes", then we should (IMO) be rounding to the nearest minute, not just truncating seconds. Can we make that happen?

::: toolkit/components/mozintl/test/test_mozintl.js:78
(Diff revision 4)
> +    testRTFBestUnit(anchor, "2016-04-10 12:01:59", "in 1 minute");
> +    testRTFBestUnit(anchor, "2016-04-10 12:59:59", "in 59 minutes");

These seem a bit counter-intuitive to me; I'd have expected the :59 seconds to be rounded up so we'd get "in 2 minutes" and "in 1 hour" as "best-fit" results.

::: toolkit/components/mozintl/test/test_mozintl.js:87
(Diff revision 4)
> +    testRTFBestUnit(anchor, "2016-04-10 13:59:59", "in 1 hour");
> +    testRTFBestUnit(anchor, "2016-04-10 23:59:59", "in 11 hours");

Similarly, ISTM that a difference of +1hr 59m 59s is much better displayed as "in 2 hours", and likewise "in 12 hours" for the 23:59:59 example.

(But I haven't thought through exactly how this would be implemented, and what other changes it might imply...)

Comment 22

[rxaviers]

Hi Jonathan,

Zibi brought your question to my attention. At first glance by looking at these specific test cases, I would have the same impression as you had, i.e., it seems like the continuous distance (based on seconds or milliseconds) would be better. The problem is that the continuous distance can lead into incorrect results, specially the cases where we use textual output such as "now", "today", "yesterday", etc. Visual examples can be found here: https://github.com/tc39/proposal-intl-relative-time/issues/14

Relative time (in this library) is defined as what makes sense for expressions like "now", "2 days ago", "in 3 months", "last year", "yesterday", and so on. In a more formal definition, relative time is an approximate date distance given a unit. This is, relative time is the date distance of a and b ± error, where error < unit. According to this definition, the test cases above stress the extreme cases (where error is almost the unit itself), hence the feeling it's incorrect, but it's valid.

A more intuitive way of thinking about the test case above is to think of a clock that doesn't display seconds (since we're going to use `minute` as precision unit). Let's say you have a meeting that starts at "2:00 PM" and your laptop clock displayes "1:45 PM". You would expect to see "in 15 minutes" regardless of how far you're from 45 (e.g., 1:45:59). If you see "in 14 minutes", there's an impression your meeting will start at "1:59 PM". For other units, shown in https://github.com/tc39/proposal-intl-relative-time/issues/14 and in https://github.com/rxaviers/relative-time#appendix, that's even more obvious.

Comment 23

[rxaviers]

displays*

Comment 24

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Putting it back on Jonathan's review queue based on Rafael's response.

Comment 25

[Jonathan Kew [:jfkthame]]

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

https://reviewboard.mozilla.org/r/188098/#review234548

Thanks for the response, Rafael. That helps to clarify the thinking here.

Sorry for the delay, Zibi; I've been out for a couple of weeks. I guess it's fine to go forward here, modulo minor nits.

::: toolkit/components/mozintl/mozIntl.js:201
(Diff revision 4)
> +        var diff = {
> +          _: {},
> +          ms: date.getTime() - now.getTime(),
> +          years: date.getFullYear() - now.getFullYear()
> +        };
> +        var round = Math[diff.ms > 0 ? "floor" : "ceil"];

`round` seems like a confusing name, as it doesn't end up doing a rounding operation. Anyhow, can't you just use `Math.trunc()` instead?

::: toolkit/components/mozintl/mozIntl.js:250
(Diff revision 4)
> +          case "year": return this.format(diff.years, unit);
> +          case "month": return this.format(diff.months, unit);
> +          case "day": return this.format(diff.days, unit);
> +          case "hour": return this.format(diff.hours, unit);
> +          case "minute": return this.format(diff.minutes, unit);
> +          default: return this.format(diff.seconds, unit);

As a sanity-check, could we assert that `unit` is "second" in the default case? There shouldn't be any other possibility...

::: toolkit/components/mozintl/test/test_mozintl.js:143
(Diff revision 4)
> +function test_rtf_formatBestUnit_old() {
> +  // testRTFBestUnit(0, "now");
> +  // testRTFBestUnit(-0, "now");

What's the story with all the commented-out tests here?

Comment 26

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188098/diff/4-5/

Comment 27

[Jonathan Kew [:jfkthame]]

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

https://reviewboard.mozilla.org/r/188098/#review234900

::: toolkit/components/mozintl/mozIntl.js:251
(Diff revisions 4 - 5)
> +        }
>          super(getLocales(locales), options, ...args);
>        }
>  
>        formatBestUnit(date, {now = new Date()} = {}) {
> -        var diff = {
> +        const diff = {

It seems counter-intuitive to declare this as `const`, when you're then going to immediately modify it (by adding a bunch of getters)!

Maybe that's usual JS style? Not my language, I'm just trying to read it... and it looks odd!

::: toolkit/components/mozintl/mozIntl.js:273
(Diff revisions 4 - 5)
>          });
>          defineCachedGetter(diff, "seconds", function() {
> -          return round((startOf(date, "second") - startOf(now, "second")) / second);
> +          return Math.trunc((startOf(date, "second") - startOf(now, "second")) / second);
>          });
>  
> -        var absDiff = {
> +        const absDiff = {

As above, it's declared `const` and then modified by a series of `defineGetter()` calls... feels odd.

Comment 28

[Jonathan Kew [:jfkthame]]

OK, having looked to see what `const` actually means here, I guess it's fine.... ignore me. :)

Comment 29

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

Comment on attachment 8917090 [details]
Bug 1407240 - Add mozIntl.RelativeTimeFormat.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188098/diff/5-6/

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/909ff88d91d6
Add mozIntl.RelativeTimeFormat. r=jfkthame

Comment 31

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/909ff88d91d6

Comment 32

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

Thank you Jonathan for all of the review! It's been a loong process! I'll document the API together with other mozIntl APIs in bug 1438687.

Comment 34

[Martijn]

I just stumbled across this:
https://developers.google.com/web/updates/2018/10/intl-relativetimeformat

So it's landing in Chrome 71.

I don't quite understand why this issue is marked as resolved/fixed, because I'm trying this API in Firefox DE 64, and it's not there. Maybe it needs to be enabled by some flag, in which case: please enable by default, as it's not in the way of anything.

Comment 35

[Jonathan Kew [:jfkthame]]

(In reply to Martijn from comment #34)
> I just stumbled across this:
> https://developers.google.com/web/updates/2018/10/intl-relativetimeformat

AFAICS, that's a significantly different proposal than the [internal] API that was implemented here for use in the Firefox front-end.

Comment 36

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

Actually, it's just a wrapper on top of the one that is aligned with ECMA402 and in SpiderMonkey. (see bug 
1270140). We'll expose the SpiderMonkey one once we update ICU to 63 (bug 1499026).