Closed Bug 1002575 Opened 8 years ago Closed 8 years ago

Show "Last synced" time in Remote Tabs panel

Categories

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

All
Android
defect
Not set
normal

Tracking

(firefox32 verified, firefox33 verified)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox32 --- verified
firefox33 --- verified

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(7 files, 1 obsolete file)

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.
Attached image screenshot.png
Screenshot from Chrome for comparision.
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.
(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.
(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.
Attached image remote_tabs_before.png
Before, phone.
Attached image remote_tabs_after.png
After, phone.
Attached image remote_tabs_tablet_before.png (obsolete) —
Before, tablet.
After, tablet.
Before, tablet.
Attachment #8414033 - Attachment is obsolete: true
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.
Attachment #8414044 - Flags: review?(rnewman)
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 ;)
Attachment #8414045 - Flags: review?(rnewman)
n.b., this does not address the fact that device names are still hard to identify (i.e., doesn't fix Bug 899643).
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.
Attachment #8414045 - Flags: review?(rnewman) → review+
Attachment #8414044 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/mozilla-central/rev/47f3a02e649c
https://hg.mozilla.org/mozilla-central/rev/ef914add28ca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Verified as fixed in builds:
- 33.0a1 (2014-06-18);
- 32.0a2 (2014-06-18);
Device: Google Nexus 10 (Android 4.4.2).
Will this be uplifted to 31 Beta?
Status: RESOLVED → VERIFIED
Product: Android Background Services → Firefox for Android
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.