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

Pinned Tabs are not showing on new tab page #12461

Closed
u1traviolet opened this issue Nov 22, 2022 · 8 comments
Closed

Pinned Tabs are not showing on new tab page #12461

u1traviolet opened this issue Nov 22, 2022 · 8 comments
Labels
Bug 🐞 This is a bug with existing functionality not behaving as expected

Comments

@u1traviolet
Copy link

u1traviolet commented Nov 22, 2022

Steps to reproduce

go to a webpage, click on the hamburger menu, and press 'Add to Shortcuts'

Expected behavior

pinned tab should appear on new tab page

Actual behavior

no pinned tab on new tab page

Device & build information

  • Device: iPhone SE 3 iOS 16.1.1
  • Build version: Firefox 107.1 (22302)

Notes

Attachments:

┆Issue is synchronized with this Jira Task

@u1traviolet u1traviolet added the Bug 🐞 This is a bug with existing functionality not behaving as expected label Nov 22, 2022
@dnarcese
Copy link
Contributor

dnarcese commented Dec 1, 2022

@u1traviolet Does the tab ever show up on your homepage? Is this only happening when you open an existing homepage or also when you open a new tab?

@u1traviolet
Copy link
Author

@u1traviolet Does the tab ever show up on your homepage? Is this only happening when you open an existing homepage or also when you open a new tab?

Hi, a tab I use a lot will turn up on my homepage, I can then pin it as normal by long pressing the icon. This about pinning a page I don't have on my homepage yet

@dnarcese
Copy link
Contributor

dnarcese commented Dec 1, 2022

@u1traviolet sorry thats what I meant. After you click 'Add to Shortcuts' does it ever show up, maybe the next time you open the app?

@lmarceau
Copy link
Contributor

lmarceau commented Dec 1, 2022

I can reproduce in both production and developer version of the app. Once it's not showing, it's never showing in the shortcuts (restarting the app has no effect). Haven't looked into the cause of this yet, but it's easily reproducible at least.

@u1traviolet
Copy link
Author

@u1traviolet sorry thats what I meant. After you click 'Add to Shortcuts' does it ever show up, maybe the next time you open the app?

no it does not. i've even uninstalled the app and re-downloaded it, it still happens. What happens when you try it?

@tarikeshaq
Copy link
Contributor

tarikeshaq commented Dec 4, 2022

The problem is that in

sql.getSites(forURLs: [url.absoluteString]).bind { val -> Success in
guard let site = val.successValue?.asArray().first?.flatMap({ $0 }) else {
return succeed()
}
return self.profile.history.addPinnedTopSite(site)
}.uponQueue(.main) { result in

We sql.getSites() - which in

let sql = """
SELECT history.id AS historyID, history.url AS url, title, guid, iconID, iconURL, iconDate, iconType, iconWidth
FROM view_favicons_widest, history
WHERE history.id = siteID AND history.url IN (\"\(inExpression)\")

Tries to join with the old favicon table - but since we no longer use that table, there is a good chance the site doesn't exist in the favicon table and the call returns an empty list of sites.

We should probably remove the call to getSites altogether since we already have the URL and title at that point, and it seems to me the call was purely used to get favicon data - in #12420 I do that, but that's not gonna land for a bit - I'll create another PR for this because it's easy enough 😄

Edit: I checked this out because I familiarized myself with those surfaces over the past few weeks and I was worried it was something I did 😬

@u1traviolet
Copy link
Author

Excellent, glad to know it's going to be addressed.

@data-sync-user
Copy link
Collaborator

➤ Simion Basca commented:

Verifying as fix on 109 (24441)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐞 This is a bug with existing functionality not behaving as expected
Projects
None yet
Development

No branches or pull requests

5 participants