Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude download, redirects, reload, embed and framed link visit types from the get_top_frecent_site_infos query #3503

Closed
gabrielluong opened this issue Aug 18, 2020 · 2 comments · Fixed by #3505
Assignees

Comments

@gabrielluong
Copy link
Member

gabrielluong commented Aug 18, 2020

In implementing top frecent sites for Fenix mozilla-mobile/fenix#10342 and mozilla-mobile/android-components#7978, we encountered cases where we would get redirects as part of the results from get_top_frecent_site_infos

pub fn get_top_frecent_site_infos(db: &PlacesDb, num_items: i32) -> Result<TopFrecentSiteInfos> {

In order to avoid that, we can add a exclude_types similar to get_visit_infos in

exclude_types: VisitTransitionSet,

┆Issue is synchronized with this Jira Task
┆Sprint: Backlog

@gabrielluong gabrielluong self-assigned this Aug 18, 2020
@mhammond
Copy link
Member

Do we really need it as a param? I don't really object to it being a param, but I can't see a use-case for the value of that param ever changing, and we could consider adding it as a param once we find the first use-case for needing it to be specified?

@gabrielluong
Copy link
Member Author

Do we really need it as a param? I don't really object to it being a param, but I can't see a use-case for the value of that param ever changing, and we could consider adding it as a param once we find the first use-case for needing it to be specified?

I think that sounds reasonable to hard code the exclusion type into the query. Right now, we're looking at the visited pages that are listed in the "History" screen in Fenix. This is the current exclusion types we're looking to pass in
https://github.com/mozilla-mobile/fenix/blob/8d1bd10e6b00d270cfde13719fa91a8d8f6da7a3/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt#L36.

@gabrielluong gabrielluong changed the title Add exclude_types to get_top_frecent_site_infos API Add allowed_types in get_top_frecent_site_infos query Aug 18, 2020
@gabrielluong gabrielluong changed the title Add allowed_types in get_top_frecent_site_infos query Exclude download, redirects, reload, embed and framed link visit types from the get_top_frecent_site_infos query Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants