Closed Bug 1409208 Opened 7 years ago Closed 6 years ago

Provide users on disconnect an option to delete local data

Categories

(Firefox :: Sync, enhancement, P2)

Product:
Firefox
Component:
Sync
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 62
Tracking Flags:
Tracking Status
relnote-firefox --- 62+
firefox62 --- fixed

People

(Reporter: adavis, Assigned: markh)

References

(Depends on 2 open bugs,
URL
)

Details

(Whiteboard: [Sync Q2 2018 OKR] )

Attachments

(4 files)

disconnect-delete.gif
62.74 KB, image/gif
Details
Bug 1409208 (part 1) - Create a functional disconnect dialog.
59 bytes, text/x-review-board-request
eoger
: review+
zbraniecki
: review+
Details
Bug 1409208 (part 2) - implement disconnect and sanitize functionality.
59 bytes, text/x-review-board-request
eoger
: review+
lina
: review+
Details
This is what the dialog looks like with the attached patches.
27.33 KB, image/png
Details 
See: https://bugzilla.mozilla.org/show_bug.cgi?id=1325271#c16

Upon disconnecting from Sync, many users believe that their data will delete from the browser. 

I think we have 3 options to make the experience better (and more private):

1) we display a warning about this and perhaps as a phase 1, we link to Preferences where they can pick what they would like to delete.

2) we link to a SUMO article that explains how to do it.

3) perhaps we offer to delete History, passwords, form data and all the rest (?).
See Also: → 1409209
See Also: → 1409211
See Also: → 1183693
Summary: Warn users on disconnect of Sync that their data will not be deleted from the device → [meta] Warn users on disconnect of Sync that their data will not be deleted from the device
Blocks: 1325271
See Also: 1325271 →
Summary: [meta] Warn users on disconnect of Sync that their data will not be deleted from the device → Warn users on disconnect of Sync that their data will not be deleted from the device
From meeting, we do that already (but we don't say much):
https://imgur.com/a/vhoo4
Ryan and Alex, can you please get together on the UI for this? Maybe create a feature doc for this.
Flags: needinfo?(rfeeley)
Flags: needinfo?(adavis)
Priority: -- → P2
Clearing ni? since I'm working on user switching which will hopefully cover this.
https://docs.google.com/document/d/1smeS4qlXdmJ1EKcT_wbGhlN7FOipM4TAXum5pkuDy3c/edit#
Flags: needinfo?(adavis)
Priority: P2 → --
Flags: needinfo?(amckay)
Moving into the backlog until we've can move it up in priority.
Flags: needinfo?(amckay)
Priority: -- → P3
Attached image disconnect-delete.gif — 
Copying the Clear Data dialog, Meridel and I are wondering if something like this will do the trick.
Flags: needinfo?(rfeeley)
I love this :rfeeley ! <3
Does device in "the device" mean user is disconnecting their device or other device(connected with Sync).
(In reply to In from comment #8)
> Does device in "the device" mean user is disconnecting their device or other
> device(connected with Sync).

This bug is for local disconnect. Not for remote. 

Note: Remote disconnect already recommends to do a remote wipe via OS if the device was lost of stolen.
Summary: Warn users on disconnect of Sync that their data will not be deleted from the device → Provide users on disconnect an option to delete local data
Priority: P3 → --
I think markh meant to put this as p2.
Priority: -- → P2
Whiteboard: [Sync Q2 2018 OKR]
Assignee: nobody → markh
Status: NEW → ASSIGNED
Depends on: 1457071
See Also: → 1457815
Does it also have the other state? See animation>
https://bug1409208.bmoattachments.org/attachment.cgi?id=8951722

Oddly Bugzilla is not showing the second frame to me when previewed in overlay on this page.
Flags: needinfo?(markh)
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review246540

looks great!
Attachment #8971921 - Flags: review?(gandalf) → review+
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

Flod - do you think we should attempt to salvage anything here from the old strings? Seems like not much value, but wanted to run via you.
Attachment #8971921 - Flags: feedback?(francesco.lodolo)
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review246546

::: browser/locales/en-US/browser/preferences/syncDisconnect.ftl:9
(Diff revision 1)
> +
> +sync-disconnect-dialog =
> +    .title = Disconnect Sync?
> +    .style = width: 36em; min-height: 35em;
> +
> +sync-disconnect-heading = Do you also want to remove browser data on this computer? Your data will remain in your account, regardless.

(non native speaker) Doesn't "on" sound off here, compared to "from"?

::: browser/locales/en-US/browser/preferences/syncDisconnect.ftl:11
(Diff revision 1)
> +    .title = Disconnect Sync?
> +    .style = width: 36em; min-height: 35em;
> +
> +sync-disconnect-heading = Do you also want to remove browser data on this computer? Your data will remain in your account, regardless.
> +
> +sync-disconnect-remove-sync-caption = Remove Firefox Sync Data

This should be { -sync-brand-name }

::: browser/locales/en-US/browser/preferences/syncDisconnect.ftl:19
(Diff revision 1)
> +
> +sync-disconnect-remove-other-caption = Remove Other Private Data
> +
> +sync-disconnect-remove-other-data = Cookies, cache, offline website data, etc.
> +
> +// |sync-disconnect-confirm-disconnect-delete| and |sync-disconnect-confirm-disconnect|

Wrong comment sygil

Also, it would make sense to add a section comment for these two buttons, otherwise the comment will be associated only to the first string.
Hit "Publish" too soon…

> > +sync-disconnect-remove-sync-caption = Remove Firefox Sync Data
> 
> This should be { -sync-brand-name }

To clarify: 

sync-disconnect-remove-sync-caption = Remove { -sync-brand-name } Data

And you'll need to include browser/branding/sync-brand.ftl

> > +// |sync-disconnect-confirm-disconnect-delete| and |sync-disconnect-confirm-disconnect|
> 
> Wrong comment sygil
> 
> Also, it would make sense to add a section comment for these two buttons,
> otherwise the comment will be associated only to the first string.

Correct sigil: #
Section comment: ##
http://projectfluent.org/fluent/guide/comments.html

While // probably still works, is also discontinued.
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review246554

::: browser/locales/en-US/browser/preferences/syncDisconnect.ftl:20
(Diff revision 1)
> +sync-disconnect-remove-other-caption = Remove Other Private Data
> +
> +sync-disconnect-remove-other-data = Cookies, cache, offline website data, etc.
> +
> +// |sync-disconnect-confirm-disconnect-delete| and |sync-disconnect-confirm-disconnect|
> +// are for the confirm button - only one of which will be shown at a time.

Oh, yes, please, move those two Messages below `sync-disconnect-cancel` and turn them into a section:


```

## Confirm Buttons
##
## Only one of the two buttons is shown
## at the time

sync-disconnect-confirm-disconnect-delete =
    .label = Disconnect & Delete
    .accesskey = D

sync-disconnect-confirm-disconnect =
    .label = Just Disconnect
    .accesskey = D

```
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

LTGM with the issues fixed. I agree, no point in creating a migration to salvage this little content.
Attachment #8971921 - Flags: feedback?(francesco.lodolo) → feedback+
Ryan, there was a concern raised with the wording on that dialog:

"""
Do you also want to remove browser data on this computer? Your data will remain in your account, regardless.

...

[ ] Remove Other Private Data
    Cookies, cache, offline website data, etc.

"""

Note that this other data will *not* remain in your account regardless. If you delete this local data it will be gone forever. There's a concern that users may take the wording at face value then be upset when they find this data does not come back after a reconnect.

Do you think there is something we should do to clarify this. Off the top of my head, maybe that second line could read something like "Cookies, cache, offline website data, etc. This data will be permanently removed and can not be restored" or similar.

Secondly, re Flod's suggestion:

"Do you also want to remove browser data *on* this computer?" or "Do you also want to remove browser data *from* this computer?"

Thanks!
Flags: needinfo?(rfeeley)
(In reply to Ryan Feeley [:rfeeley] from comment #15)
> Does it also have the other state? See animation>
> https://bug1409208.bmoattachments.org/attachment.cgi?id=8951722
> 
> Oddly Bugzilla is not showing the second frame to me when previewed in
> overlay on this page.

Yes, the button changes caption when either of the check-boxes are selected as per your animation.
Flags: needinfo?(markh)
Comment on attachment 8971922 [details]
Bug 1409208 (part 2) - implement disconnect and sanitize functionality.

https://reviewboard.mozilla.org/r/240660/#review246522

Looks good, thanks, Mark! I have some questions, so I wouldn't mind seeing this patch again, but feel free to drop any that don't make sense, and I'll r+.

::: browser/components/preferences/in-content/syncDisconnect.js:15
(Diff revision 2)
> +});
> +
> +
> +var gSyncDisconnectDialog = {
> +  lockRetryInterval: 2000, // wait 2 seconds before trying for the lock again.
> +  lockRetryCount: 60, // Try 60 times before giving up in disgust.

This seems a bit high, especially if the sync is taking a while. Do you think it's worth showing some more explicit UI feedback to the user, in addition to disabling the buttons?

I wonder if we should, generalize https://searchfox.org/mozilla-central/rev/08df4e6e11284186d477d7e5b0ae48483ecc979c/services/sync/modules/engines.js#1833,1835,1838,1842 to check if Sync is still enabled, and abort if not.

On the one hand, there's probably no point to finishing the sync if we're starting over. On the other, interrupting a sync halfway through could be bad, especially if we're telling folks they can sign back in and everything will still be there.

::: browser/components/preferences/in-content/syncDisconnect.js:32
(Diff revision 2)
> +  // mocked by tests.
> +  getWeave() {
> +    return ChromeUtils.import("resource://services-sync/main.js", {}).Weave;
> +  },
> +
> +  async doSantizeSync() {

I wonder if we want a shutdown blocker for this? Blocking shutdown on network requests (if we're still syncing) seems iffy, though.

Maybe we could instead race the lock promise with `quit-application-granted`, and go ahead with sanitizing without waiting for the lock? (For the Internet cafe case we talked about over Vidyo, it seems better to miss uploading some local data, than to leave it on the device).

Or maybe I'm really overthinking this, and everything is fine the way it is. :-)

::: browser/components/preferences/in-content/syncDisconnect.js:43
(Diff revision 2)
> +    try {
> +      // We clobber data for all Sync engines that are enabled.
> +      await weave.Service.promiseInitialized;
> +      weave.Service.enabled = false;
> +
> +      // We might be syncing - poll for up to 2 minutes waiting for the lock.

Oh, interesting, do we not handle this correctly today? IIUC, `Service.startOver()` doesn't take the lock, so, if you disconnect FxA in the middle of the sync, will we just make a mess of things today?

::: browser/components/preferences/in-content/syncDisconnect.js:63
(Diff revision 2)
> +        };
> +        checkLock();
> +      });
> +
> +      try {
> +        for (let engine of weave.Service.engineManager.getAll()) {

This *almost* looks like `weave.Service.wipeClient()`, save for the `enabled` check and error handling.

Do you think it's worth lifting this logic into `wipeClient`, and using that here, or would you prefer to keep them separate?

::: browser/components/preferences/in-content/syncDisconnect.js:84
(Diff revision 2)
> +    } catch (ex) {
> +      log.error("Failed to sanitize Sync data", ex);
> +      console.error("Failed to sanitize Sync data", ex);
> +    }
> +    try {
> +      // ensure any logs we wrote a flushed to disk.

Micro-nit: s/a/are?

::: browser/components/preferences/in-content/syncDisconnect.js:98
(Diff revision 2)
> +      // sanitize everything other than "open windows" (and we don't do that
> +      // because (a) it may confuse the user - they probably want to see
> +      // about:prefs with the disconnection reflected and (b) because the
> +      // actual disconnection is done by the opening window, it doesn't
> +      // happen if it is closed as part of the sanitize process.
> +      let itemsToClear = Object.keys(Sanitizer.items);

Micro-nit: `Object.keys(...).filter(...)` is shorter. :-)
Attachment #8971922 - Flags: review?(kit)
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review247020

I've played with the window and very long translations, couldn't find anything wrong with it. Thank you!

::: browser/themes/shared/incontentprefs/syncDisconnect.css:5
(Diff revision 1)
> +/* Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#SyncDisconnectDialog {

Nit: Lowercase first letter
Attachment #8971921 - Flags: review?(eoger) → review+
Comment on attachment 8971922 [details]
Bug 1409208 (part 2) - implement disconnect and sanitize functionality.

https://reviewboard.mozilla.org/r/240660/#review247024

LGTM

::: browser/components/preferences/in-content/syncDisconnect.js:131
(Diff revision 2)
> +      console.error("Failed to sanitize", ex);
> +    }).finally(() => {
> -    // We dispatch a custom event so the caller knows how we were closed
> +      // We dispatch a custom event so the caller knows how we were closed
> -    // (if we were a dialog we'd get this for free, but we'd also lose the
> +      // (if we were a dialog we'd get this for free, but we'd also lose the
> -    // ability to keep the dialog open while the sanitize was running)
> +      // ability to keep the dialog open while the sanitize was running)
> -    let closingEvent = new CustomEvent("dialogclosing", {
> +      let closingEvent = new CustomEvent("dialogclosing", {

I don't see anywhere we handle that event, but I assume we do this because of:
https://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/subdialogs.js#178
Attachment #8971922 - Flags: review?(eoger) → review+
Quick fix: replace "Your data" with "Firefox Sync data".
Flags: needinfo?(rfeeley)
Thanks for pushing back on that patch Kit - it wasn't ready to land.

> This seems a bit high, especially if the sync is taking a while. Do you
> think it's worth showing some more explicit UI feedback to the user, in
> addition to disabling the buttons?

The UI now shows a "Disconnecting..." and a spinner while the delete is happening.

> I wonder if we should, generalize
> https://searchfox.org/mozilla-central/rev/
> 08df4e6e11284186d477d7e5b0ae48483ecc979c/services/sync/modules/engines.
> js#1833,1835,1838,1842 to check if Sync is still enabled, and abort if not.
> 
> On the one hand, there's probably no point to finishing the sync if we're
> starting over. On the other, interrupting a sync halfway through could be
> bad, especially if we're telling folks they can sign back in and everything
> will still be there.

That does seem a little heavy-handed TBH, and it's really not clear if it's better to abort (meaning their data might not be on the server) or to abort, so I didn't bother with that - but let me know if you really think it is important (and see below - it should always delete data on shutdown)

> I wonder if we want a shutdown blocker for this? Blocking shutdown on
> network requests (if we're still syncing) seems iffy, though.
> 
> Maybe we could instead race the lock promise with
> `quit-application-granted`, and go ahead with sanitizing without waiting for
> the lock? (For the Internet cafe case we talked about over Vidyo, it seems
> better to miss uploading some local data, than to leave it on the device).

Yes, good idea! There's now a shutdown blocker that does roughly what you suggest - once the blocker kicks in we immediately abort waiting for the sync lock and start the sanitize and disconnect steps.

Note also that the core sanitize and disconnect functionality has now been moved into a module - this addresses other problems with the old patch: (a) the user closing the tab or dialog while waiting for the lock would prevent the sanitize happening and (b) if the user attempted to sanitize twice (eg, after accidentally closing) things would almost certainly go pear-shaped.

> Or maybe I'm really overthinking this, and everything is fine the way it is.

As above, I don't think you are and thanks!

> Oh, interesting, do we not handle this correctly today? IIUC,
> `Service.startOver()` doesn't take the lock, so, if you disconnect FxA in
> the middle of the sync, will we just make a mess of things today?

Yeah - and that remains true in my patch - if you don't select "sanitize sync data" we still don't wait for the lock - but I think that's probably OK given we have no reports of bad things happening, and those bad things would be limited to Sync getting a little confused re auth and aborting with an error - but as the user has no expectation of data being removed, this seems fine.

(The reason I took the lock before deleting was simply the risk that a sync running may add more data after the deletion, which would be bad).

> This *almost* looks like `weave.Service.wipeClient()`, save for the
> `enabled` check and error handling.
> 
> Do you think it's worth lifting this logic into `wipeClient`, and using that
> here, or would you prefer to keep them separate?

I've a couple of issues with that - mainly that it doesn't seem to honour the engine enabled state, and it also fails to remove data if the client can't decrypt (for reasons I don't understand) - I think we want to avoid both those cases and I'm a little too scared to change wipeClient at this time :)

All other nits addressed, and a new patch is forthcoming.
For QA, here's a summary of this feature and some ideas for testing:

Feature Description:
* When disconnecting a device from Sync via about:preferences, we will show UI that offers 2 additional options as well as the actual disconnection:
1) Delete the data that Sync manages on this device.
2) Delete other browser data from the device.

* They may select either, or both options. If no options are selected, the browser data is not touched, but we still disconnect the FxA account (or they may cancel the operation completely, in which case we do nothing.

If they select option (1), then we remove all data in the profile that is associated with their Sync data. For example, if the user previousl chose to sync Bookmarks, but not History, then all bookmarks will be removed but history will not be touched.

If they select option (2), then almost all browser data is removed - it is exactly equivalent to opening about:preferences#privacy, selecting "Clear History", then choosing "Erase Everything" and leaving every option selected. Note that this means there's quite some overlap between (1) and (2) - for example, assuming the user is syncing bookmarks, their bookmarks will be deleted if *either* option is selected. Conversely, if the user is *not* syncing History, then option (1) will not remove history, but option (2) will. This overlap is intentional.

In both cases, the browser should be disconnected from Firefox Accounts - all Firefox UI should appear as though they have never signed in to Sync.

Test Ideas:
* Obviously the core functionality as described above should work - make sure the browser has sync data and other non-sync data, and ensure the correct data is removed as expected from the options chosen, and that the browser really ends up disconnected from Sync.

* If a Sync is currently in progress, the process will wait for up to 2 minutes until it is complete before deleting Sync data, with the dialog showing a "Disconnecting..." spinner. If they close this window or the entire tab, the dialog will close, but it will continue to wait before deleting.

* If the dialog is re-opened in this period, it will just show the "Disconnecting..." spinner instead of re-asking what should be removed.

*If the entire browser is terminated before this Sync completes, the data will be immediately deleted at shutdown.

* The browser will continue to show the user as being logged in while waiting for a sync to complete - the Sync disconnection is the last step in the process.

* For all the above though, note that most Syncs are very quick - it will be quite tricky to arrange for a Sync to take long enough to test the above. There is a trick I can share to help test this using the "Browser Toolbox":

** If you execute in the console the command "Weave.Service.lock()", and it returns "true", then you've effectively tricked the system into thinking a Sync is running, so all the points above will apply (eg, the "Disconnecting..." status will be shown for up to 2 minutes, etc)  Note that if the lock() call returns false, then it almost certainly means a "real" sync is running - you should let that finish and try again.

** You can execute "Weave.Service.unlock()" in the console to let the system believe the fake-sync we did above has completed and should be enough to let the disconnect process complete. You can never call this to check that we only wait 2 minutes and disconnect anyway, or to check that we do disconnect on browser shutdown.
Comment on attachment 8971922 [details]
Bug 1409208 (part 2) - implement disconnect and sanitize functionality.

https://reviewboard.mozilla.org/r/240660/#review250636

::: services/sync/modules/SyncDisconnect.jsm:111
(Diff revision 3)
> +  // Sanitize all Browser data.
> +  async doSanitizeBrowserData() {
> +    try {
> +      // sanitize everything other than "open windows" (and we don't do that
> +      // because (a) it may confuse the user - they probably want to see
> +      // about:prefs with the disconnection reflected and (b) because the

(b)'s not true any more
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review250640

::: browser/components/preferences/in-content/syncDisconnect.js:13
(Diff revision 2)
> +
> +  // when either of the checkboxes are changed.
> +  onDeleteOptionChange() {
> +    let eitherChecked = document.getElementById("deleteRemoteSyncData").checked ||
> +                        document.getElementById("deleteRemoteOtherData").checked;
> +    let newTitle = eitherChecked ? "sync-disconnect-confirm-disconnect-delete" :

remove space

::: browser/components/preferences/in-content/syncDisconnect.js:39
(Diff revision 2)
> +      // We dispatch a custom event so the caller knows how we were closed
> +      // (if we were a dialog we'd get this for free, but we'd also lose the
> +      // ability to keep the dialog open while the sanitize was running)
> +      let closingEvent = new CustomEvent("dialogclosing", {
> +        bubbles: true,
> +        detail: { button: "accept" },

remove spaces

::: browser/locales/en-US/browser/preferences/syncDisconnect.ftl:29
(Diff revision 2)
> +    .accesskey = C
> +
> +## Disconnect confirm Button
> +##
> +## The 2 labels which may be shown on the single "Disconnect" button, depending
> +## on the state of ther checkboxes.

ther->the
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review251622

::: browser/components/preferences/in-content/syncDisconnect.js:32
(Diff revision 2)
> +    document.getElementById("deleteOptionsContent").hidden = true;
> +    document.getElementById("deletingContent").hidden = false;
> +
> +    // And do the santize.
> +    promiseComplete.catch(ex => {
> +      console.error("Failed to sanitize", ex);

Should we still close the window (or unblock the UI) if this happens?
Comment on attachment 8971921 [details]
Bug 1409208 (part 1) - Create a functional disconnect dialog.

https://reviewboard.mozilla.org/r/240658/#review251622

> Should we still close the window (or unblock the UI) if this happens?

TBH I'm not sure what we'd show - the error is almost certainly not suitable for showing to the user and just unblocking without showing why seems worse. This should not happen in practice and if it does, it's almost certainly going to be during shutdown where we've lost the opportunity to do much. So I think this isn't ideal, but is OK.
Comment on attachment 8971922 [details]
Bug 1409208 (part 2) - implement disconnect and sanitize functionality.

https://reviewboard.mozilla.org/r/240660/#review250594

::: browser/components/preferences/in-content/sync.js:15
(Diff revision 3)
>  
>  XPCOMUtils.defineLazyGetter(this, "FxAccountsCommon", function() {
>    return ChromeUtils.import("resource://gre/modules/FxAccountsCommon.js", {});
>  });
>  
> +XPCOMUtils.defineLazyGetter(this, "SyncDisconnect", function() {

Nit: Could we use `ChromeUtils.defineModuleGetter` here?

::: services/sync/modules/SyncDisconnect.jsm:54
(Diff revision 3)
> +        if (attempts >= this.lockRetryCount) {
> +          log.error("Gave up waiting for the sync log - going ahead with sanitize anyway");
> +          resolve(false);
> +          return;
> +        }
> +        log.debug("Waiting a couple of seconds to get the sync log");

Micro-nit: s/log/lock, here and above.

::: services/sync/modules/SyncDisconnect.jsm:139
(Diff revision 3)
> +  // Start the sanitization process. Returns a promise that resolves when
> +  // the sanitize is complete, and another "resolve" function which can be
> +  // called to abort the process of waiting for a sync to complete which
> +  // should help the finished promise resolve quicker.
> +  _startDisconnect({sanitizeSyncData = false, sanitizeBrowserData = false} = {}) {
> +    // This is a bit convoluted - we want to wait for a sync to finish before

This almost like a good place to use `AbortController` here (return the controller, and use the signal to resolve the promise)...but I think the comment makes it clear what's going on after a couple of reads, and does the same thing without introducing another object, so I'll leave it to you if you want to try it out, or keep it as-is.

::: services/sync/modules/SyncDisconnect.jsm:175
(Diff revision 3)
> +    this.promiseDisconnectFinished = promiseDisconnectFinished;
> +    let shutdownBlocker = () => {
> +      // oh dear - we are sanitizing (probably stuck waiting for a sync to
> +      // complete) and the browser is shutting down. Let's avoid the wait
> +      // for sync to complete and continue the process anyway.
> +      promiseNotSyncingAbortResolve(false);

Maybe `abortWaitingForSyncLock`, or something like that, would be clearer?
Attachment #8971922 - Flags: review?(kit) → review+
See Also: → 1463956
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/9edaa56a21d4
(part 1) - Create a functional disconnect dialog. r=eoger,gandalf
https://hg.mozilla.org/integration/autoland/rev/1c6cfeb419c3
(part 2) - implement disconnect and sanitize functionality. r=eoger,kitcambridge
https://hg.mozilla.org/mozilla-central/rev/9edaa56a21d4
https://hg.mozilla.org/mozilla-central/rev/1c6cfeb419c3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Depends on: 1466982
Depends on: 1466983
Depends on: 1466986
Depends on: 1466814
Depends on: 1467928
Would you like this to have a release note for 62? If so, can you suggest wording?
Flags: needinfo?(markh)
Sync Privacy/Security Improvement:
Users who disconnect Firefox for desktop from Sync are now offered the option to wipe their personal data from that device. (e.g. bookmarks, passwords, history, cookies, site date, etc)
Flags: needinfo?(markh) → needinfo?(lhenry)
Noted for beta 62 as: Users who disconnect Firefox for desktop from Sync are now offered the option to wipe their personal data from that device (such as bookmarks, passwords, history, cookies, and site data)
relnote-firefox: --- → 62+
Flags: needinfo?(lhenry)
Perfect! Thanks
I drafted a SUMO article for this feature. Can you please take a look and add any suggestions by Aug 20 at the latest? https://docs.google.com/document/d/1trD0vtF5SIcA7dH07XeFRwi5dFjEOZJWC-1SK6Yn-pE/edit?usp=sharing
Flags: needinfo?(adavis)
Thanks Joni. This looks good. Attached to this bug is an animated gif that could potentially be useful for the SUMO article.

My only suggestion for the article is to add that visual.
Flags: needinfo?(adavis)
Depends on: 1483866
Depends on: 1483017
Depends on: 1539784
No longer depends on: 1539784
Regressions: 1539784
Depends on: 1557894
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: