1470043 - Add a Places maintenance task to remove items with invalid URLs

Status: NEW

(Toolkit :: Places, enhancement, P3)

Description

[Lina Butler [:lina]]

A user ran into this in https://support.mozilla.org/en-US/questions/1222500. Invalid item URLs are almost certainly going to cause problems elsewhere, not just in Sync, and we should remove them.

We probably can't do this for all invalid URLs in `moz_places`, since that would require a full table scan without an index. Starting with items seems reasonable, though.

Does this sound like a good idea to you, Mak, or should we ask users to manually rebuild their databases if an invalid URL sneaks in?

Comment 1

[Phabricator Automation]

Attachment: Add a Places maintenance task to remove items with invalid URLs.

Comment 2

[Mark Banner (:standard8)]

Note, we also have bug 458619 and bug 1401401 that relate to this. The latter is mainly suggesting a maintenance task.

Comment 3

[Marco Bonardo [:mak]]

The problem is that to do this we must scan the whole database, even if it's just for bookmarks, we have users with ten thousands or more.
I was rather thinking to do this from some calling code, like when we fail creating an nsIURI/URL in certain code points, we invoke an API to cleanup the thing. I didn't have a chance to investigate doing that though.

Comment 4

[Perry Wagle]

What's the definition of an invalid URL?  Like what would you say in the documentation?

Comment 5

[Perry Wagle]

I hope that this would not be automatic, I'd like to be given a chance to repair the url before all the other info in the bookmark is erased?

But yeah, the original problem of causing issues in sync seems serious and urgent?

Comment 6

[Marco Bonardo [:mak]]

(In reply to Perry Wagle from comment #5)

I hope that this would not be automatic, I'd like to be given a chance to repair the url before all the other info in the bookmark is erased?

That's a valid concern.
A simple approach may be to export a firefox_invalid_bookmarks.txt (or json) to Desktop with the removed uris, and notify about that.
A more complex approach may be to move them into a separate table, then after startup if the table is non empty present a notification to the user with a button pointing to a privileged content page to fix them. If no action is taken for N sessions, just throw the url away.
The implementation cost is surely different here, that makes me think approach 1 is more likely.