Add removeClientData to Service

RESOLVED DUPLICATE of bug 565430

Status

P5
normal
RESOLVED DUPLICATE of bug 565430
8 years ago
6 years ago

People

(Reporter: mlissner+bugzilla, Assigned: rnewman)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.7) Gecko/20100715 Ubuntu/10.04 (lucid) Firefox/3.6.7
Build Identifier: 

I recently got rid of one of my computers, but bits and pieces of it are still around in the form of things that were synced via Weave.

For example, I currently have ten links in my "Tabs from other computers" list from a computer that I no longer have....and for the life of me, I can't figure out how to get rid of them, short of editing things on the weave server.

I may be missing something, but I posted it here: http://support.mozilla.com/en-US/forum/1/721484?s=weave+remove+computer&as=s

And so far no responses...

Reproducible: Always

Steps to Reproduce:
1. Sync a computer with weave.
2. Never turn it on again.
Actual Results:  
Tabs from that computer persist...seemingly forever.

Expected Results:  
Some method to remove the computer from Weave.

I suppose this could have been avoided by closing that browser completely first, or something from that computer...but I no longer have that option.
Firefox Sync must support removing computer from the synced computer list.
# it's better if we can hide from specific computer, not only remove from
# all the computer I think (but maybe it should be another bug...)

This problem also happen not only when we stop using some computer but also when we have some trouble with sync and re-setup it on the same computer.
If we have some trouble with one of the computer we are using Firefox Sync, currently we cannot remove the obsolete computer from the "Tabs from other computers" list.

This problem sometimes cause privacy concerns. If we cannot remove (or hide) the computer from the "Tabs from other computers" list, we cannot hide the open tab in private computer at office/school.
Status: UNCONFIRMED → NEW
Ever confirmed: true
BTW, Michael, here is a workaround for this:
 1. Reset Sync on any computer you are now using sync.
 2. Re-Set up Sync with the option:
    "Replace all other devices with your local data"

If we replace the whole data on the server with one of your computer's latest data, all computer will be removed and only computers you continue using will be shown in the list.

Note: of course, this workaround is not acceptable for general users. We must support computer list management feature.
(Reporter)

Comment 3

8 years ago
@dynamis, that's a great work around, duh! Thanks. It seems to work well!
Morphing this: In short, delete client-specific data for any engines that use that.  We should expose a property on each engine (including clients) that indicates if that engine operates on a per-client basis.  For each engine where this property is true, issue a delete for storage/<collection>/<clientId>.

This is required for bug 565430, adding dependency.
Blocks: 565430
OS: Linux → All
Hardware: x86 → All
Summary: Weave lacks a method for removing synced tabs and computers → add removeClientData to Service
Target Milestone: --- → 1.6
(Assignee)

Updated

8 years ago
Priority: -- → P5
Summary: add removeClientData to Service → Add removeClientData to Service
Target Milestone: 1.6 → ---
(Assignee)

Updated

8 years ago
Assignee: nobody → rnewman
(Assignee)

Comment 5

8 years ago
Created attachment 522068 [details] [diff] [review]
WIP Part 1. v1

This part:

* Adds a test for removeClientData, verifying that the appropriate DELETE gets called for input collections.

* Implements said method.

* Adds a _perClientCollections attribute to SyncEngine, which is ["tabs"] for TabEngine. This is not explicitly tested as part of this patch.

Bug 565430 is probably the right place for the obvious Part 2.
Attachment #522068 - Flags: review?(philipp)
(Assignee)

Comment 6

8 years ago
Comment on attachment 522068 [details] [diff] [review]
WIP Part 1. v1

Actually, let's hold off on the review for now. Need to think about the error handling strategy a little more.
Attachment #522068 - Attachment description: Part 1. v1 → WIP Part 1. v1
Attachment #522068 - Flags: review?(philipp)
(Assignee)

Comment 7

8 years ago
Created attachment 522538 [details] [diff] [review]
Part 1. v2

This includes the followup, too; it's still pretty compact.
Attachment #522068 - Attachment is obsolete: true
Attachment #522538 - Flags: review?(philipp)
(Assignee)

Comment 8

8 years ago
Tests (including the new test) pass. I verified that "Deactivate this device" resulted in DELETEs being issued in a test profile.
Whiteboard: [has patch][needs review philikon]
(Assignee)

Comment 9

8 years ago
STR for QA:

* Set up sync. Enable sync logging. Restart.
* Choose "deactivate this device" in Sync prefs.
* Verify that DELETE for tabs and clients appears in the log.

For extra points, set up the usual pair of clients to observe "Tabs from other computers", and perform these steps, observing that the deactivated device no longer appears in TFOC after a sync.
Comment on attachment 522538 [details] [diff] [review]
Part 1. v2

Apart from the fact that perClientCollections being an array seems unnecessary (each engine corresponds to exactly one collection), I don't think Service should do the work. Clean up is an engine specific process, the engine should do it.

As per RL discussion, we should introduce SyncEngine.prototype.startOver as a no-op, have Service.startOver call all the engine's startOver methods. Then the client engine and tabs engine can issue their one delete.
Attachment #522538 - Flags: review?(philipp) → review-
Since I don't think this is the right approach (comment 10), I'm duping this to bug 565430.
No longer blocks: 565430
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 565430
Whiteboard: [has patch][needs review philikon]
You need to log in before you can comment on or make changes to this bug.