Closed Bug 599544 Opened 9 years ago Closed 9 years ago

Need notification for when storage format mismatch


(Firefox for Android Graveyard :: General, defect)

Not set



Tracking Status
fennec 2.0b2+ ---


(Reporter: philikon, Assigned: mfinkle)



(Whiteboard: [strings])


(1 file, 1 obsolete file)

Once in a while a newer Firefox Sync version may require a newer storage format, making it incompatible with older versions. There are two scenarios here:

* User upgrades a desktop computer to a newer Firefox Sync version. The desktop will reupload everything in the new storage format. The outdated Fennec client should display a notification upon the next sync telling the user to upgrade to a newer version of Fennec's Sync (or Fennec).

* User ugprades a mobile device to a newer Firefox Sync version. The data in the account is still in the old format. Key generation is disabled on Fennec, so Sync won't reupload everything. It should display a notification that tells the user to upgrade all of their other computers as well and have them sync (the first desktop machine will reupload everything in the new format).
Not sure of the right milestone for this to block, but it should absolutely block something, hopefully b1?
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0+
Renominating this for blocking-fennec to block 4.0b2. The crypto simplification that's being planned for Sync will rev the storage format once more so the sooner the have Fennec message a storage format mismatch the better.

I don't know how exactly the notification would be displayed, but the mechanics of it would be similar to how it's done in Firefox [1]:

* listen to the weave:service:sync:error observer notification

* check for Weave.Status.sync == Weave.VERSION_OUT_OF_DATE and each of Weave.Status.engines.* == Weave.VERSION_OUT_OF_DATE.

tracking-fennec: 2.0+ → ?
(In reply to comment #2)
> * check for Weave.Status.sync == Weave.VERSION_OUT_OF_DATE and each of
> Weave.Status.engines.* == Weave.VERSION_OUT_OF_DATE.

To be clear here, if any of those are true, we should show the notification.
Attached patch patch (obsolete) — Splinter Review
Simple patch that uses the info provided and some code from Firefox to display a simple alert, telling the user Sync needs to be updated. Then we load the "update" URL in a new tab.

I don't know if this will give the user enough explanation as to what's happening. I reused the strings from the Firefox notification. Are they applicable to Fennec?

CC'ing Madhava too
Assignee: nobody → mark.finkle
Attachment #484946 - Flags: review?(mconnor)
tracking-fennec: ? → 2.0b2+
How about "Update Firefox to continue using Sync" ?

Ideally, wherever we show this would give users an avenue to kick off an update.
Attached patch patch 2Splinter Review
This patch uses VERSION_OUT_OF_DATE and DESKTOP_VERSION_OUT_OF_DATE to display two different strings, and the option to visit a web page to learn more about the problem.
Attachment #484946 - Attachment is obsolete: true
Attachment #485444 - Flags: review?(mconnor)
Attachment #484946 - Flags: review?(mconnor)
Attachment #485444 - Flags: ui-review?(madhava)
Attachment #485444 - Flags: review?(mbrubeck)
Are we giving people a way to update - is that something that's on the Learn More page?  I don't want to make people hunt for it themselves.

Also - one quibble about the remote version.  Wouldn't it be better to say "Please update Firefox on your other computer(s)." - it's not clear to me what it would mean to update sync, when sync is built in.
Attachment #485444 - Flags: review?(mbrubeck) → review+
Comment on attachment 485444 [details] [diff] [review]
patch 2

>+    // Check for a storage format update, update the user and load the Sync update page
>+    if (aTopic =="weave:service:sync:error") {
>+      let clientOutdated = Weave.Status.sync == Weave.VERSION_OUT_OF_DATE;
>+      for (let [engine, reason] in Iterator(Weave.Status.engines))
>+        clientOutdated = clientOutdated || reason == Weave.VERSION_OUT_OF_DATE;
>+      let remoteOutdated = Weave.Status.sync == Weave.DESKTOP_VERSION_OUT_OF_DATE;
>+      for (let [engine, reason] in Iterator(Weave.Status.engines))
>+        remoteOutdated = remoteOutdated || reason == Weave.DESKTOP_VERSION_OUT_OF_DATE;

I know this is last minute, but this is a lot of unnecessary work.  Why not do something like this? 

  let clientOutdated = false, remoteOutdated = false;
  if (Weave.Status.sync == Weave.VERSION_OUT_OF_DATE)
    clientOutdated = true;
  else if (Weave.Status.sync == Weave.DESKTOP_VERSION_OUT_OF_DATE)
    remoteOutdated = true;
  else if (Weave.Status.service == Weave.SYNC_FAILED_PARTIAL) {
    // some engines failed, check for per-engine compat
    for (let [engine, reason] in Iterator(Weave.Status.engines)) {
       clientOutdated = clientOutdated || reason == Weave.VERSION_OUT_OF_DATE;
       remoteOutdated = remoteOutdated || reason == Weave.DESKTOP_VERSION_OUT_OF_DATE;

with that, r=me

Also, to answer madhava's question, the intent is that the page will support client/local/remote info as params so we can make this smarter over time.  Sync is built into Firefox, but we're building against a future where this may not just be a Firefox feature, so we need to make sure this is flexible.
Attachment #485444 - Flags: review?(mconnor) → review+
Whiteboard: [strings]
pushed with mconnor's code change and madhava's string change:
Closed: 9 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: in-litmus?
Need older version of Fennec to verify...
Flags: in-litmus? → in-litmus?(tchung)
Flags: in-litmus?(tchung) → in-litmus+
Attachment #485444 - Flags: ui-review?(madhava)
You need to log in before you can comment on or make changes to this bug.