Closed Bug 664590 Opened 9 years ago Closed 2 years ago

History sync: ignore file:/// URIs

Categories

(Firefox :: Sync, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: philikon, Assigned: eoger)

References

(Blocks 1 open bug)

Details

(Whiteboard: [sync:history])

Attachments

(1 file)

file:/// URIs are very likely not going to make sense on other computers. They only clutter up awesomebar results.
Something to consider as we move towards some kind of "profile in the cloud", or more durable single-device 'backup' solutions — we have to think about somehow storing data that only a single client cares about. We don't want to sync them, but …
Whiteboard: [sync:history]
Blocks: 745408
Bumping this so we see it in our weekly triage.
I think this is a valuable bug to fix, we can either filter the file:// uris in the tracker [0] or before syncing [1]

[0] https://searchfox.org/mozilla-central/rev/a5d613086ab4d0578510aabe8653e58dc8d7e3e2/services/sync/modules/engines/history.js#380,400
[1] https://searchfox.org/mozilla-central/rev/a5d613086ab4d0578510aabe8653e58dc8d7e3e2/services/sync/modules/engines/history.js#78
Assignee: nobody → eoger
Priority: -- → P2
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment on attachment 8935139 [details]
Bug 664590 - Do not track file:/// visits.

https://reviewboard.mozilla.org/r/205980/#review211626

I think we probably need a similar change in getAllIDs (which probably means using a constant or function to work out if a URL qualifies). We might also want to consider not applying incoming records that meet the criteria (ie, over time, this change should end up meaning no file:// URLs end up on the server, but we might as well stop the ones that are already there being applied locally).
Attachment #8935139 - Flags: review?(markh)
N.B., for devices that are using Sync as a backup, this is data loss — you sync, you restore, and you are missing bits of history.

I would think very carefully about whether it's better to have URIs that you've visited but can't use on this particular device, or whether it's better to add this complexity so that some history dies with a particular profile.

It's also worth thinking: if file: URIs aren't backed up to the cloud, should they be saved in history at all?

FWIW, I've never heard a user complain about this; Ed's Comment 2 implies that he sees value in the change, but I wanted to capture that there are tradeoffs here.
Thanks Richard, I agree there are tradeoffs and my opinions are weakly held.

(In reply to Richard Newman [:rnewman] from comment #5)
> N.B., for devices that are using Sync as a backup, this is data loss — you
> sync, you restore, and you are missing bits of history.

Yeah, but we try to discourage users seeing this as a backup because it's quite possible it will fail to actually do this for them if they are a single device user.

OTOH, from the POV of a multi-device Sync user, this is introducing garbage on their other profiles.

> I would think very carefully about whether it's better to have URIs that
> you've visited but can't use on this particular device, or whether it's
> better to add this complexity so that some history dies with a particular
> profile.

I think it all hinges on the word "backup".

> It's also worth thinking: if file: URIs aren't backed up to the cloud,
> should they be saved in history at all?

Again, the word "backup" :) If we ever had something designed to backup a profile to the cloud, then yes, we should also include those visits. But Sync is not that.

> FWIW, I've never heard a user complain about this; Ed's Comment 2 implies
> that he sees value in the change, but I wanted to capture that there are
> tradeoffs here.

By the same token, I can't really imagine users complaining if they did manage to successfully use Sync as a backup service and got all history back other than file:// URIs.

FWIW, my history has many file:// URLs pointing at Sync logs and so are even more tightly bound to a profile rather than a device.

I have personally seen this behaviour (eg, a file URI pointing at, roughly %TEMP%/test.html be synced to my other devices) and just thought "heh - silly sync" rather than filed a bug - so I don't feel strongly either way. But I don't think the case for syncing them is stronger than the case for not syncing them.

There's also moz-extension URLs that we probably want to blacklist in the same way - there's no bug on file for history, but bug 1412083 asks for us not to sync prefs that have them.
Thanks Mark, I amended my patch.
I also think that if we insist so much on telling users that Sync is not a backup solution, we might as well ignore these local visits.
Comment on attachment 8935139 [details]
Bug 664590 - Do not track file:/// visits.

https://reviewboard.mozilla.org/r/205980/#review214042

Looks great, thanks!
Attachment #8935139 - Flags: review?(markh) → review+
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d57aa14966b9
Do not track file:/// visits. r=markh
https://hg.mozilla.org/mozilla-central/rev/d57aa14966b9
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.