Use plural form for reader mode article reading time strings

VERIFIED FIXED in Firefox 53

Status

()

Toolkit
Reader Mode
VERIFIED FIXED
6 months ago
3 months ago

People

(Reporter: Gijs, Assigned: fiveNinePlusR)

Tracking

(Blocks: 1 bug)

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 months ago
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?
Flags: needinfo?(emanuela)
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.
(Assignee)

Comment 2

6 months ago
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?
Assignee: nobody → fiveNinePlusR
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(emanuela)
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"?
(Reporter)

Comment 4

6 months ago
(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.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(francesco.lodolo)
(Reporter)

Comment 5

6 months ago
(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
(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.
Flags: needinfo?(francesco.lodolo)
(Assignee)

Comment 7

6 months ago
Created attachment 8819621 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch
Attachment #8819621 - Flags: review?(gijskruitbosch+bugs)
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
(Reporter)

Comment 9

6 months ago
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...
Attachment #8819621 - Flags: review?(gijskruitbosch+bugs) → feedback+
Summary: Add localization comments for reader mode article reading time strings → Use plural form for reader mode article reading time strings
(Assignee)

Comment 10

6 months ago
Created attachment 8819986 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch
Attachment #8819621 - Attachment is obsolete: true
Attachment #8819986 - Flags: review+
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.
(Assignee)

Comment 12

6 months ago
Created attachment 8819990 [details] [diff] [review]
0001-Bug-1323415-Add-reader-mode-time-estimate-pluralizat.patch
Attachment #8819986 - Attachment is obsolete: true
Attachment #8819990 - Flags: review+
(Assignee)

Updated

6 months ago
Attachment #8819990 - Flags: review+ → review?(gijskruitbosch+bugs)
(Reporter)

Comment 13

6 months ago
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!
Attachment #8819990 - Flags: review?(gijskruitbosch+bugs) → review+
(Reporter)

Updated

6 months ago
Keywords: checkin-needed

Comment 14

6 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e3eeb479512
Add reader mode time estimate pluralization strings. r=gijs
Keywords: checkin-needed

Comment 15

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7e3eeb479512
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 16

4 months ago
Verified on Aurora 53.0a2, with several devices. The estimated reading time uses the correct plural strings.
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
Blocks: 1347460
You need to log in before you can comment on or make changes to this bug.