1002575 - Show "Last synced" time in Remote Tabs panel

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Android Sync, defect)

Description

[Nick Alexander :nalexander [he/him]]

I don't know if this is feasible, but my guess is it is.  We should be able to at least surface the lastModified timestamp from each remote client record.

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: screenshot.png

Screenshot from Chrome for comparision.

Comment 2

[Richard Newman [:rnewman]]

We can definitely surface when *we* last synced.

We can also surface the modified time of each remote *tab* record (not client record), which should get bumped every time a remote client syncs.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #2)
> We can definitely surface when *we* last synced.


Explain why?  I know the Android client uploads a new client record every sync.  Arguably, that's not right.  Is this not the case for the desktop client?  Not part of the Sync "contract"?

> We can also surface the modified time of each remote *tab* record (not
> client record), which should get bumped every time a remote client syncs.

Agreed.  I don't want to surface per-tab times, however.  I'd be happy surfacing the max of each client's tab records, though, as an approximation to "last synced" as I described it.

Comment 4

[Richard Newman [:rnewman]]

(In reply to Nick Alexander :nalexander from comment #3)

> Explain why?  I know the Android client uploads a new client record every
> sync.  Arguably, that's not right.  Is this not the case for the desktop
> client?  Not part of the Sync "contract"?

Indeed. The only requirement (inasmuch as the behavior of the clients collection is specified at all) is that clients upload a new record before the old one TTLs away. IIRC, that means desktop uploads every two weeks, because the TTL is three weeks.


> > We can also surface the modified time of each remote *tab* record (not
> > client record), which should get bumped every time a remote client syncs.
> 
> Agreed.  I don't want to surface per-tab times, however.  I'd be happy
> surfacing the max of each client's tab records, though, as an approximation
> to "last synced" as I described it.

There's only one tabs record per client, with the key of each being the client ID. We upload all of your open tabs every time we sync.

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: remote_tabs_before.png

Before, phone.

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: remote_tabs_after.png

After, phone.

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: remote_tabs_tablet_before.png

Before, tablet.

Comment 8

[Nick Alexander :nalexander [he/him]]

Attachment: remote_tabs_tablet_after.png

After, tablet.

Comment 9

[Nick Alexander :nalexander [he/him]]

Attachment: remote_tabs_tablet_before.png

Before, tablet.

Comment 10

[Nick Alexander :nalexander [he/him]]

Attachment: Part 1: Expose client lastModified in TabsAccessor. r=rnewman

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: Part 2: Display "Last synced" time in Synced tabs panel. r=rnewman

For the resources, I kept the client/last_synced text sizes the same as
the tab/url text sizes; and I kept the client text size the size of the
previous group (since I didn't want to calculate the new group size);
and I stripped the (unnecessary) backgrounds.

Comment 12

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8414045 [details] [diff] [review]
Part 2: Display "Last synced" time in Synced tabs panel. r=rnewman

So, how scared are you of deferring to Android's DateUtils?  I can't see any locale problems now or in the future ;)

Comment 13

[Nick Alexander :nalexander [he/him]]

n.b., this does not address the fact that device names are still hard to identify (i.e., doesn't fix Bug 899643).

Comment 14

[Richard Newman [:rnewman]]

Comment on attachment 8414045 [details] [diff] [review]
Part 2: Display "Last synced" time in Synced tabs panel. r=rnewman

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

DateUtils appears to be locale-switching safe.

::: mobile/android/base/RemoteTabsList.java
@@ +128,5 @@
> +     * @return string describing time span
> +     */
> +    protected String getLastSyncedString(long now, long time) {
> +        CharSequence relativeTimeSpanString = DateUtils.getRelativeTimeSpanString(time, now, DateUtils.MINUTE_IN_MILLIS);
> +        return getResources().getString(R.string.remote_tabs_last_synced, relativeTimeSpanString);

I'm not sure whether this is l10n-friendly -- we're reliant on the output of getRelativeTimeSpanString, and that might not be adequate in every locale.

For example, consider a language in which time nouns are declensed. Imagine that "1 week ago" was "femaleweek" or "maleweek" depending on whether the event that occurred was male or female. DateUtils always returns "maleweek". Oh dear.

That's probably not something we can handle at all using Android's DateUtils. Roll on l20n.

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +437,5 @@
> +
> +<!-- Localization note (remote_tabs_last_synced): the variable is replaced by a
> +     "relative time span string" produced by Android.  This string describes the
> +     time the tabs were last synced relative to the current time; examples
> +     include "42 minutes ago", "4 days ago", "last week", etc. -->

Add:

  The subject of "Last synced" is one of the user's other Sync clients, typically Firefox on their desktop or laptop.

Comment 15

[Nick Alexander :nalexander [he/him]]

Pushed with nits.

https://hg.mozilla.org/integration/fx-team/rev/47f3a02e649c
https://hg.mozilla.org/integration/fx-team/rev/ef914add28ca

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/47f3a02e649c
https://hg.mozilla.org/mozilla-central/rev/ef914add28ca

Comment 17

[Flaviu Cos]

Verified as fixed in builds:
- 33.0a1 (2014-06-18);
- 32.0a2 (2014-06-18);
Device: Google Nexus 10 (Android 4.4.2).

Comment 18

[Flaviu Cos]

Will this be uplifted to 31 Beta?