AS highlights: Implement domain blacklist

RESOLVED WONTFIX

Status

()

Firefox for Android
General
P2
normal
RESOLVED WONTFIX
a year ago
6 months ago

People

(Reporter: sebastian, Assigned: ahunt)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MobileAS])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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
(Reporter)

Updated

a year ago
Blocks: 1311089
(Reporter)

Updated

a year ago
Priority: P3 → P2

Updated

a year ago
See Also: → bug 1314718
(Assignee)

Comment 1

a year 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

a year ago
Depends on: 1324051
(Assignee)

Comment 2

a year 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

a year ago
Attachment #8820009 - Flags: review?(gkruglov)
(Assignee)

Comment 4

a year 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

Updated

11 months ago
Iteration: 1.12 → 1.13
(Assignee)

Updated

11 months 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

11 months ago
Iteration: 1.13 → ---
(Reporter)

Updated

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