Closed Bug 602715 Opened 9 years ago Closed 9 years ago

Sync UI: Only mention enabled engines in client wipe confirmation dialog

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: dholbert, Assigned: philikon)

References

Details

(Whiteboard: [see comment #7][strings?])

Attachments

(3 files)

STR:
 1. Open Sync Preferences
 2. Under "Sync My", check only one item (e.g. bookmarks), and uncheck everything else
 3. Click "Manage Account" and then "Reset Sync"
 4. Choose "Replace all data on this computer with my Sync data", and click Next
 5. Sync brings up a very scary dialog saying:

"All Minefield data on this computer will be deleted, including the following:
   XXX bookmarks
   YYY days of history
   ZZZ passwords
 WARNING: This will result in all Minefield data on this computer being replaced!"

Truthfully, only the options I've checked (in this case, bookmarks) will be replaced -- everything else is NOT replaced.
blocking2.0: --- → ?
Summary: When I've only set up Sync to only handle e.g. bookmarks "Reset Sync" lies and tells me it's going to delete my passwords → When I've only set up Sync to only handle e.g. bookmarks, "Reset Sync" lies and tells me it's going to delete my passwords
Note that fixing this would require string changes (I think?), so if any action were to happen on it, it'd need to happen soon, since beta7 is the string freeze.
I don't think this needs string changes, unless I'm missing something.
I was assuming that we might want to change e.g.
> All $Program data on this computer will be deleted, including the following
...to something like:
> Your $SyncEngine1, $SyncEngine2, and $SyncEngine3 data on this
> computer will be deleted, including the following
...which would be a string change.  But maybe we can make it clearer while still using existing strings -- I don't know.
(where $SyncEngine1, $SyncEngine2, $SyncEngine3 represent the boxes that are ticked in the "Sync my..." settings)
...and a similar change to the string at the end of the dialog (" WARNING: This will result in all Minefield data on this computer being replaced!")

The key thing is that "all Minefield data" in that string is not correct. (and the degree to which it's incorrect depends on how many categories the user is set up to sync)
Yep, that's scary and incorrect. Blocking b7+ so we can figure out if this actually requires string changes or not. Sync people, can you take a look as soon as possible, maybe get some feedback from UX on the right wording?

If it's not possible or desired to get this b7 even if requires string changes, then ping a driver to push the flag to a later release.
blocking2.0: ? → beta7+
Whiteboard: [see comment #7][strings?]
Talked this over with dietrich - I am loathe to contemplate a break to the string freeze, but we don't know if this will need one or not, and we need to get b7 out the door. Moving this to beta8, still need the sync team to chime in ASAP, and cc'ng l10n preemptively.
blocking2.0: beta7+ → beta8+
Summary: When I've only set up Sync to only handle e.g. bookmarks, "Reset Sync" lies and tells me it's going to delete my passwords → When I've set up Sync to only handle e.g. bookmarks, "Reset Sync" lies and tells me it's going to delete my passwords
Assignee: nobody → philipp
Summary: When I've set up Sync to only handle e.g. bookmarks, "Reset Sync" lies and tells me it's going to delete my passwords → Sync UI: Only mention enabled engines in client wipe confirmation dialog
Attached patch v1Splinter Review
* Only list data of enabled engines in the client wipe confirmation dialog.
* Also mention preferences if they're enabled.
* Got rid of the redundant warnings.
Attachment #488317 - Flags: review?(mconnor)
Attachment #488317 - Flags: feedback?(l10n)
Attached image screenshot of v1
Attachment #488318 - Flags: feedback?(faaborg)
Attachment #488317 - Flags: feedback?(l10n) → feedback+
Attachment #488318 - Flags: feedback?(faaborg) → feedback+
Comment on attachment 488317 [details] [diff] [review]
v1

># HG changeset patch
># Parent f7016571b4726d9f465ef4b698f0e9e88adaa3ea
># User Philipp von Weitershausen <philipp@weitershausen.de>
>Bug 602715 - Sync UI: Only mention enabled engines in client wipe confirmation dialog
>
>diff --git a/browser/base/content/syncSetup.js b/browser/base/content/syncSetup.js
>--- a/browser/base/content/syncSetup.js
>+++ b/browser/base/content/syncSetup.js
>@@ -757,65 +757,78 @@
>   _handleChoice: function () {
>     let desc = document.getElementById("mergeChoiceRadio").selectedIndex;
>     document.getElementById("chosenActionDeck").selectedIndex = desc;
>     switch (desc) {
>       case 1:
>         if (this._case1Setup)
>           break;
> 
>-        // history
>-        let db = Weave.Svc.History.DBConnection;
>+        let places_db = Weave.Svc.History.DBConnection;
>+        if (Weave.Engines.get("history").enabled) {
>+          let daysOfHistory = 0;
>+          let stm = places_db.createStatement(
>+            "SELECT ROUND(( " +
>+              "strftime('%s','now','localtime','utc') - " +
>+              "( " +
>+                "SELECT visit_date FROM moz_historyvisits " +
>+                "UNION ALL " +
>+                "SELECT visit_date FROM moz_historyvisits_temp " +
>+                "ORDER BY visit_date ASC LIMIT 1 " +
>+                ")/1000000 " +
>+              ")/86400) AS daysOfHistory ");
> 
>-        let daysOfHistory = 0;
>-        let stm = db.createStatement(
>-          "SELECT ROUND(( " +
>-            "strftime('%s','now','localtime','utc') - " +
>-            "( " +
>-              "SELECT visit_date FROM moz_historyvisits " +
>-              "UNION ALL " +
>-              "SELECT visit_date FROM moz_historyvisits_temp " +
>-              "ORDER BY visit_date ASC LIMIT 1 " +
>-              ")/1000000 " +
>-            ")/86400) AS daysOfHistory ");
>+          if (stm.step())
>+            daysOfHistory = stm.getInt32(0);
>+          // Support %S for historical reasons (see bug 600141)
>+          document.getElementById("historyCount").value =
>+            PluralForm.get(daysOfHistory,
>+                           this._stringBundle.GetStringFromName("historyDaysCount.label"))
>+                      .replace("%S", daysOfHistory)
>+                      .replace("#1", daysOfHistory);
>+        } else {
>+          document.getElementById("historyCount").hidden = true;
>+        }
> 
>-        if (stm.step())
>-          daysOfHistory = stm.getInt32(0);
>-        // Support %S for historical reasons (see bug 600141)
>-        document.getElementById("historyCount").value =
>-          PluralForm.get(daysOfHistory,
>-                         this._stringBundle.GetStringFromName("historyDaysCount.label"))
>-                    .replace("%S", daysOfHistory)
>-                    .replace("#1", daysOfHistory);
>+        if (Weave.Engines.get("bookmarks").enabled) {
>+          let bookmarks = 0;
>+          let stm = places_db.createStatement(
>+            "SELECT count(*) AS bookmarks " +
>+            "FROM moz_bookmarks b " +
>+            "LEFT JOIN moz_bookmarks t ON " +
>+            "b.parent = t.id WHERE b.type = 1 AND t.parent <> :tag");
>+          stm.params.tag = Weave.Svc.Bookmark.tagsFolder;
>+          if (stm.executeStep())
>+            bookmarks = stm.row.bookmarks;
>+          // Support %S for historical reasons (see bug 600141)
>+          document.getElementById("bookmarkCount").value =
>+            PluralForm.get(bookmarks,
>+                           this._stringBundle.GetStringFromName("bookmarksCount.label"))
>+                      .replace("%S", bookmarks)
>+                      .replace("#1", bookmarks);
>+        } else {
>+          document.getElementById("bookmarkCount").hidden = true;
>+        }
> 
>-        // bookmarks
>-        let bookmarks = 0;
>-        stm = db.createStatement(
>-          "SELECT count(*) AS bookmarks " +
>-          "FROM moz_bookmarks b " +
>-          "LEFT JOIN moz_bookmarks t ON " +
>-          "b.parent = t.id WHERE b.type = 1 AND t.parent <> :tag");
>-        stm.params.tag = Weave.Svc.Bookmark.tagsFolder;
>-        if (stm.executeStep())
>-          bookmarks = stm.row.bookmarks;
>-        // Support %S for historical reasons (see bug 600141)
>-        document.getElementById("bookmarkCount").value =
>-          PluralForm.get(bookmarks,
>-                         this._stringBundle.GetStringFromName("bookmarksCount.label"))
>-                    .replace("%S", bookmarks)
>-                    .replace("#1", bookmarks);
>+        if (Weave.Engines.get("passwords").enabled) {
>+          let logins = Weave.Svc.Login.getAllLogins({});
>+          // Support %S for historical reasons (see bug 600141)
>+          document.getElementById("passwordCount").value =
>+            PluralForm.get(logins.length,
>+                           this._stringBundle.GetStringFromName("passwordsCount.label"))
>+                      .replace("%S", logins.length)
>+                      .replace("#1", logins.length);
>+        } else {
>+          document.getElementById("passwordCount").hidden = true;
>+        }
> 
>-        // passwords
>-        let logins = Weave.Svc.Login.getAllLogins({});
>-        // Support %S for historical reasons (see bug 600141)
>-        document.getElementById("passwordCount").value =
>-          PluralForm.get(logins.length,
>-                         this._stringBundle.GetStringFromName("passwordsCount.label"))
>-                    .replace("%S", logins.length)
>-                    .replace("#1", logins.length);
>+        if (!Weave.Engines.get("prefs").enabled) {
>+          document.getElementById("prefsWipe").hidden = true;
>+        }
>+
>         this._case1Setup = true;
>         break;
>       case 2:
>         if (this._case2Setup)
>           break;
>         let count = 0;
>         function appendNode(label) {
>           let box = document.getElementById("clientList");
>diff --git a/browser/base/content/syncSetup.xul b/browser/base/content/syncSetup.xul
>--- a/browser/base/content/syncSetup.xul
>+++ b/browser/base/content/syncSetup.xul
>@@ -475,44 +475,38 @@
>       <deck id="chosenActionDeck">
>         <vbox id="chosenActionMerge" class="confirm">
>           <description class="normal">
>             &confirm.merge.label;
>           </description>
>         </vbox>
>         <vbox id="chosenActionWipeClient" class="confirm">
>           <description class="normal">
>-            &confirm.client.label;
>+            &confirm.client2.label;
>           </description>
>           <separator class="thin"/>
>           <vbox id="dataList">
>             <label class="data indent" id="bookmarkCount"/>
>             <label class="data indent" id="historyCount"/>
>             <label class="data indent" id="passwordCount"/>
>+            <label class="data indent" id="prefsWipe"
>+                   value="&engine.prefs.label;"/>
>           </vbox>
>           <separator class="thin"/>
>           <description class="normal">
>             &confirm.client.moreinfo.label;
>           </description>
>-          <separator class="thin"/>
>-          <description class="warning">
>-            &confirm.client.warning.label;
>-          </description>
>         </vbox>
>         <vbox id="chosenActionWipeServer" class="confirm">
>           <description class="normal">
>-            &confirm.server.label;
>+            &confirm.server2.label;
>           </description>
>           <separator class="thin"/>
>           <vbox id="clientList">
>           </vbox>
>-          <separator class="thin"/>
>-          <description class="warning">
>-            &confirm.server.warning.label;
>-          </description>
>         </vbox>
>       </deck>
>   </wizardpage>
> 
>   <wizardpage label="&setup.successPage.title;" 
>               id="successfulSetup"
>               onextra1="gSyncSetup.onSyncOptions()"
>               onpageshow="gSyncSetup.onPageShow()">
>diff --git a/browser/locales/en-US/chrome/browser/syncSetup.dtd b/browser/locales/en-US/chrome/browser/syncSetup.dtd
>--- a/browser/locales/en-US/chrome/browser/syncSetup.dtd
>+++ b/browser/locales/en-US/chrome/browser/syncSetup.dtd
>@@ -87,18 +87,16 @@
> <!ENTITY choice2.merge.main.label      "Merge this computer's data with my &syncBrand.shortName.label; data">
> <!ENTITY choice2.merge.recommended.label "Recommended:">
> <!ENTITY choice2.client.main.label     "Replace all data on this computer with my &syncBrand.shortName.label; data">
> <!ENTITY choice2.server.main.label     "Replace all other devices with this computer's data">
> 
> <!-- Confirm Merge Options -->
> <!ENTITY setup.optionsConfirmPage.title "Confirm">
> <!ENTITY confirm.merge.label    "&syncBrand.fullName.label; will now merge all this computer's browser data into your Sync account.">
>-<!ENTITY confirm.client.label         "All &brandShortName; data on this computer will be deleted, including the following:">
>+<!ENTITY confirm.client2.label         "Warning: The following &brandShortName; data on this computer will be deleted:">
> <!ENTITY confirm.client.moreinfo.label "&brandShortName; will then copy your &syncBrand.fullName.label; data to this computer.">
>-<!ENTITY confirm.client.warning.label "WARNING: This will result in all &brandShortName; data on this computer being replaced!">
>-<!ENTITY confirm.server.label         "The following devices will be overwritten with your local data:">
>-<!ENTITY confirm.server.warning.label "WARNING: Your local data will replace all &brandShortName; data on these devices!">
>+<!ENTITY confirm.server2.label         "Warning: The following devices will be overwritten with your local data:">
> 
> <!-- New & Existing Account: Setup Complete -->
> <!ENTITY setup.successPage.title "Setup Complete">
> <!ENTITY changeOptions.label "You can change this preference by selecting Sync Options below.">
> <!ENTITY continueUsing.label "You may now continue using &brandShortName;.">
Attachment #488317 - Flags: review?(mconnor) → review+
http://hg.mozilla.org/mozilla-central/rev/581df48b9296
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
verified with recent nightly builds
Status: RESOLVED → VERIFIED
No longer blocks: 615950
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.