AS highlights: Implement domain blacklist

RESOLVED WONTFIX

Status

()

P2
normal
RESOLVED WONTFIX
3 years ago
2 years ago

People

(Reporter: sebastian, Assigned: ahunt)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MobileAS])

Attachments

(1 attachment)

In bug 1293710 we added a basic query for loading "highlights" from the database.

The desktop version ships a blacklist of domains[1] (e.g. mail.google.com). It is using the 'rev_host' (reversed hostname) column for filtering. Such a column does not exist in the mobile database. We could try implementing such a filter using NOT LIKE syntax, but we need to make sure that this does not have a negative impact on performance.

[1] https://github.com/mozilla/activity-stream/blob/9eb9f451b553bb62ae9b8d6b41a8ef94a2e020ea/addon/PlacesProvider.js#L37
Blocks: 1311089
Priority: P3 → P2

Updated

2 years ago
See Also: → bug 1314718
(Assignee)

Comment 1

2 years ago
I imagine we can use the domain_id column (Bug 1319485) for this, I still need to think about the optimal approach though.

I wonder if we should add histogram telemetry to the highlights query, similar to what we use for the topsites panel? I might try to land that as a separate bug so we can get some data on existing performance first?
Assignee: nobody → ahunt
Iteration: --- → 1.11
Depends on: 1319485
Priority: P2 → P1
(Assignee)

Updated

2 years ago
Depends on: 1324051
(Assignee)

Comment 2

2 years ago
One "fast" approach would be to modify the domains table to have a "blacklisted" field, and then only select sites where domain_id isn't blacklisted. That would however require extra fields in the domains table (it hasn't landed yet, so we can still make this change now...), and would possibly make management of the domains table more complicated (especially since we'll need to prepopulate it with the blacklisted domains).

A separate domain_blacklist table might be useful, especially if we want to enable "dismiss all sites from this domain" style functionality - that still needs prepopulating, but management should be easier (we'd still need to update the domain cleanup triggers to handle three tables, *if* we stick with the triggers).

Or we can just stuff the list of blacklisted domains into SQL. The topsites query essentially does this for suggested sites, so it's probably not too slow?
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8820009 - Flags: review?(gkruglov)
(Assignee)

Comment 4

2 years ago
Comment on attachment 8820009 [details]
Bug 1298786 - Add hardcoded highlights domain blacklist

Cancelling review for now - the domains table is likely to change a bit more in Bug 1319485, so I might need to adjust the blacklist to match (i.e. the iOS domain extraction algorithm removes "www.", so we'll probably need to remove that in the blacklist too).
Attachment #8820009 - Flags: review?(gkruglov)
Iteration: 1.11 → 1.12
Iteration: 1.12 → 1.13
(Assignee)

Updated

2 years ago
Priority: P1 → P2
(In reply to Andrzej Hunt :ahunt from comment #2)
> One "fast" approach would be to modify the domains table…

Using the domains table for this will work so long as you never want to blacklist www.foo.com and allow m.foo.com.

Tread carefully: you might want to consider a different representation, like storing a fixed-size fast hash for a domain instead of using a table.
(Assignee)

Updated

2 years ago
Iteration: 1.13 → ---
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.