1028815 - "7 days ago" label in History panel is confusing

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Francesco Lodolo [:flod]]

I received a bug report for my localization, because the label used in the History panel was imprecise: history is from last week, label says it is from 7 days ago.

Not sure if it's just my English, but when I read "7 days ago" I think about a precise moment, not a period spanning over the last 7 days. Maybe "Last 7 days", or "Last week" would be less confusing?

String was originally introduced in bug 695307.

Comment 1

[Lucas Rocha (:lucasr)]

I agree that 'Last week' feels more accurate and clear. Ian, what do you think?

Comment 2

[Ian Barlow (:ibarlow)]

I was having a similar conversation with some people on the UX team a couple weeks ago, how our labeling doesn't feel quite right. We had come up with a bunch of alternate options, but here are a couple that I rather liked: 


Option 1
-----------------------------
* Today
* Yesterday
* Last Week
* Last Month


Option 2
-----------------------------
* Today
* Yesterday
* In the last week
* In the last month
* Older than a month


Option 3
-----------------------------
* Today
* Yesterday
* In the last 7 days
* In the last 30 days
* Older than 30 days

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Ian Barlow (:ibarlow) from comment #2)
> I was having a similar conversation with some people on the UX team a couple
> weeks ago, how our labeling doesn't feel quite right. We had come up with a
> bunch of alternate options, but here are a couple that I rather liked: 
> 
> 
> Option 1
> -----------------------------
> * Today
> * Yesterday
> * Last Week
> * Last Month
> 
> 
> Option 2
> -----------------------------
> * Today
> * Yesterday
> * In the last week
> * In the last month
> * Older than a month
> 
> 
> Option 3
> -----------------------------
> * Today
> * Yesterday
> * In the last 7 days
> * In the last 30 days
> * Older than 30 days

I like the simplicity of Option 1. flod, do you see any l10n issues in Option 1?

Comment 4

[Francesco Lodolo [:flod]]

No issue, they'll be fine for l10n (just add a comment explaining where these strings are used).

Comment 5

[Lucas Rocha (:lucasr)]

Actually, Ian, maybe we should replace "Last Month" with "Older than a week"?

Comment 6

[Aaron Train [:aaronmt]]

Is there already an open bug on reorganizing the history sections or should this be re-summarized?

Comment 7

[Ian Barlow (:ibarlow)]

(In reply to Lucas Rocha (:lucasr) from comment #5)
> Actually, Ian, maybe we should replace "Last Month" with "Older than a week"?

It's a bit wordy. It also seems unlikely to me that folks would be able to scroll further back than a month in their history anyway, without having to resort to using the awesomebar to search for something specific.

Comment 8

[Matthew]

Hi there. My name is Matt and I'm new to the Firefox community. I'm looking for a good entry point for contributing to the Firefox for Android project. I was wondering, is this bug a good entry point for me?

Comment 9

[Aaron Train [:aaronmt]]

Yes, this bug is a good entry point. The mentor, Lucas is out the next few days at Google IO; hopefully someone else can point you to some starting references.

Comment 10

[Matthew]

Great, thanks! In that case I'd be happy to have this assigned to me if someone else is willing to mentor, or if Lucas can help when he's back.

Comment 11

[Aaron Train [:aaronmt]]

Sure thing

Comment 12

[Chenxia Liu [:liuche]]

Hi Matthew! Glad to hear you're interested in this bug.

If you don't have a build set up already, you'll want to follow the instructions at https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec

Once you get a build set up, you'll just need to change the strings, which will be in mobile/android/base/locales/en-US/android_strings.dtd.

When updating existing strings, we need to update the entity names as well so that the people who localize our strings can tell that it's changed. So for example, the week string would need to be changed from:

<!ENTITY history_week_section "7 days ago">
to
<!ENTITY history_week_section2 "Last week">

That's just a rough overview of what you'd need to do for this bug, so feel free to needinfo? me on this bug, or ask in #mobile on IRC if you have any questions!

Comment 13

[Matthew]

I managed to get the build set up alright, and made the change you suggested to android_strings.dtd.

My question is where else should I change the history_week_section name?

At a minumum, that is so I can succesfully build, I have to change:
<string name="history_week_section">&history_week_section;</string>
to
<string name="history_week_section">&history_week_section2;</string>
in /mobile/android/base/strings.xml.in.

Should I append the 2 to the string in quotes and then change the variable in /mobile/android/base/home/HistoryPanel.java as well?

Comment 14

[Matthew]

Sorry, I think I entered the wrong user in the need more info section.

Comment 15

[Chenxia Liu [:liuche]]

Hi Matthew,

That looks great, you don't need to do anything more to change an existing string.

If you look at comment #2, you should also make the other string change suggested in Option #1.

If you'll upload a patch, I can review it for you. The patch message (which you can set by typing 'hg qref -e') should be the following:

Bug 1028815 - "7 days ago" label in History panel is confusing. r=liuche

(This would also be the format of any future patches, "Bug <#> - <bug title>. r=<reviewer>".)

Comment 16

[Matthew]

Attachment: bug-1028815-fix.patch

Comment 17

[Chenxia Liu [:liuche]]

Comment on attachment 8447475 [details] [diff] [review]
bug-1028815-fix.patch

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

Great.

One thing that you will want to do in the future is to set the "review" flag to "?" <reviewer>.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=1732da079963

When the try run is all green, you can put "checkin-needed" in the Keywords field, and one of the sheriffs will land the patch for you in the tree.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/c1389e23e29d

Comment 19

[Carsten Book [:Tomcat]]

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

Comment 20

[Teodora Vermesan (:TeoVermesan)]

History panel has:
* Today
* Yesterday
* Last Week
* Last Month
So:
Verified fixed on:
Device: LG Nexus 4
OS: Android 4.4.2
Build: Firefox for Android 33.0a1 (2014-07-02)

Comment 21

[Teodora Vermesan (:TeoVermesan)]

Will this be uplifted to Aurora?

Comment 22

[Francesco Lodolo [:flod]]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #21)
> Will this be uplifted to Aurora?

At the end of the cycle (Jul 21) it will move to Aurora like the rest of mozilla-central code.

Comment 23

[Francesco Lodolo [:flod]]

Also marking as Verified based on comment 20.