1323415 - Use plural form for reader mode article reading time strings

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect)

Description

[:Gijs (out for now; he/him)]

See bug 1265304 comment 39 and further.

Emanuela, can you clarify if you're OK with sticking with "min", and/or if you want a "." after it, and/or if you want "minute(s)" instead?

Comment 1

[emanuela [ux]]

My recommendation is to have 'minute/minutes' and then localize the string.

If the reading time is less then 1 minute, simply don't show it.

Comment 2

[fiveNinePlusR]

Just to clarify on this, Since we are going to show at least 1 minute there should be 3 cases with a localization note? 

```1 minute```

```4 minutes``` (The low and high estimate can be the same if the characters are within a certain range. Just did a quick test and it looks like they can be equal within the range 1-5 with the detected languages and variance so far.)

```1-2 minutes``` (typical case for English)

The pluralization rules of different cultures vary a bit from what I recall. For instance Hebrew has a pretty complicated pluralization scheme: http://www.unicode.org/cldr/charts/29/supplemental/language_plural_rules.html#he

I could amend the commit to support these cases if that's the direction we'd like to take it. 

Gijs, what do you think?

Comment 3

[Francesco Lodolo [:flod]]

Please check the current support for plural forms
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_and_Plurals

As I said, I really don't know how we should deal with a range like "1-4 minutes", given that plural forms are based only on one variable. That's also why I suggested to keep using an abbreviation.

Out of curiosity, how is the range determined? What's the advantage from the user's point of view in receiving this information compared to "x minutes"?

Comment 4

[:Gijs (out for now; he/him)]

(In reply to Francesco Lodolo [:flod] (mostly out of office until Dec 19) from comment #3)
> Please check the current support for plural forms
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/
> Localization_and_Plurals
> 
> As I said, I really don't know how we should deal with a range like "1-4
> minutes", given that plural forms are based only on one variable. That's
> also why I suggested to keep using an abbreviation.

That option would still be open to localizations if they were unable to determine the correct plural form with only, say, the highest number of the two, right? It doesn't seem like we lose anything here using a "nicer" string in the languages where we're able to do so. It's unfortunate we can't do the perfect thing with PluralForm as-is, but there we are...

> Out of curiosity, how is the range determined?

Based on a range of reading speeds per language, expressed as characters / minute.

> What's the advantage from the
> user's point of view in receiving this information compared to "x minutes"?

Well, the range of reading speeds combined with the reader's experience of being either a fast or slow reader provides more accurate information, and "8-10 minutes" is a more user-friendly description than "9 minutes +/- 1 minute", as it were.

Comment 5

[:Gijs (out for now; he/him)]

(In reply to fiveNinePlusR from comment #2)
> Just to clarify on this, Since we are going to show at least 1 minute there
> should be 3 cases with a localization note? 
> 
> ```1 minute```
> 
> ```4 minutes``` (The low and high estimate can be the same if the characters
> are within a certain range. Just did a quick test and it looks like they can
> be equal within the range 1-5 with the detected languages and variance so
> far.)
> 
> ```1-2 minutes``` (typical case for English)
> 
> The pluralization rules of different cultures vary a bit from what I recall.
> For instance Hebrew has a pretty complicated pluralization scheme:
> http://www.unicode.org/cldr/charts/29/supplemental/language_plural_rules.
> html#he
> 
> I could amend the commit to support these cases if that's the direction we'd
> like to take it. 
> 
> Gijs, what do you think?

As I understand it we want 2 strings, which in English will have 2 plural forms each. 1 string for the case where we have only 1 number, and 1 string for the case where we have two numbers. So in English I guess they look like:

readingTimeSingleEstimate = #1 minute;#1 minutes
# we never hit the singular case here - can't have "1-1 minute". Not sure if we're allowed to leave this empty...
readingTimeMultipleEstimate = ;#1-#2 minutes

Comment 6

[Francesco Lodolo [:flod]]

(In reply to :Gijs Kruitbosch from comment #4)
> That option would still be open to localizations if they were unable to
> determine the correct plural form with only, say, the highest number of the
> two, right? It doesn't seem like we lose anything here using a "nicer"
> string in the languages where we're able to do so. It's unfortunate we can't
> do the perfect thing with PluralForm as-is, but there we are...

Finally found the information I was looking for (thanks to Pike). These are the rules for ranges in Polish (Range section)
http://www.unicode.org/cldr/charts/latest/supplemental/language_plural_rules.html#pl

Sadly we don't fully support that in the current l10n system. On the other hand, units of measurements would be invariant, and that should work for abbreviations too, that's why I keep suggesting it.

If we want to use "minutes", let's make sure to use the last number (#2) as the variable to determine the plural form, and add a localization comment explaining it.

> Based on a range of reading speeds per language, expressed as characters /
> minute.

Sorry, confusing question. It should have been: how do we decide to expose the range "8-10 minutes" instead of "9 minutes"?

(In reply to :Gijs Kruitbosch from comment #5)
> # we never hit the singular case here - can't have "1-1 minute". Not sure if
> we're allowed to leave this empty...
> readingTimeMultipleEstimate = ;#1-#2 minutes

Technically we do, there's at least one string that does that, and it's extremely confusing. I would go with 

readingTimeMultipleEstimate = #1-#2 minute;#1-#2 minutes

Even if we don't support the '1' case.

Comment 7

[fiveNinePlusR]

Attachment: 0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 8819621 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Review of attachment 8819621 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/reader/AboutReader.jsm
@@ +761,3 @@
>      }
>  
> +    return PluralForm.get(fastEstimate, gStrings.GetStringFromName(displayStringKey))

You need to use slowEstimate (the larger number), and also explicit it in the localization comment.

::: toolkit/locales/en-US/chrome/global/aboutReader.properties
@@ +13,4 @@
>  aboutReader.colorScheme.sepia=Sepia
>  aboutReader.colorScheme.auto=Auto
>  
> +#LOCALIZATION NOTE (aboutReader.estimatedReadTimeValue, aboutReader.estimatedReadingTimeRange):

As explained in the doc, you need to put a specific localization comment before each plural form (copy it from the doc)
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Use_proper_plural_forms

Comment 9

[:Gijs (out for now; he/him)]

Comment on attachment 8819621 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Review of attachment 8819621 [details] [diff] [review]:
-----------------------------------------------------------------

This looks OK besides the comments flod gave and 1 more I detail below. I expect the next patch will be r+ - thanks also for the test, which is helpful! :-)

::: toolkit/locales/en-US/chrome/global/aboutReader.properties
@@ +14,5 @@
>  aboutReader.colorScheme.auto=Auto
>  
> +#LOCALIZATION NOTE (aboutReader.estimatedReadTimeValue, aboutReader.estimatedReadingTimeRange):
> +# #1 is estimated reading time value in minutes
> +aboutReader.estimatedReadTimeValue=#1 minute;#1 minutes

Note that unfortunately, now that we've landed these strings, we should iterate their string IDs/identifiers, to avoid localizers ending up with 'stale' strings for them because they already translated them (we sadly do not have a sophisticated 'did this string change' system that can propagate such changes while maintaining the same ID). So either we can come up with an entirely new identifier, or you can suffix "1" or "2" to the ID as a kind of poor person's revisioning system, which is the route we usually take...

Comment 10

[fiveNinePlusR]

Attachment: 0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Comment 11

[Francesco Lodolo [:flod]]

Comment on attachment 8819986 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Review of attachment 8819986 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/locales/en-US/chrome/global/aboutReader.properties
@@ +23,5 @@
> +# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
> +# When there is some uncertainty in how long the article will take to read show a range of
> +# minutes it is expected to take.
> +# #1 is the number of minutes it is estimated to take to read the article for a fast reader
> +# #2 is the number of minutes it is estimated to take to read the article for a slow reader

Please add that #2 is the variable used to determine the plural form.

Comment 12

[fiveNinePlusR]

Attachment: 0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Comment 13

[:Gijs (out for now; he/him)]

Comment on attachment 8819990 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch

Review of attachment 8819990 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e3eeb479512
Add reader mode time estimate pluralization strings. r=gijs

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7e3eeb479512

Comment 16

[Oana Horvath [:ohorvath]]

Verified on Aurora 53.0a2, with several devices. The estimated reading time uses the correct plural strings.