1233778 - Display website favicon beside logins in login list view

Status: RESOLVED WONTFIX

(Firefox for iOS :: Home screen, defect)

Description

[Stephan Leroux [:sleroux]]

The base implementation of the LoginTableViewCell displays the static default icon beside the login information. If there is a favicon available, we should display that instead.

Comment 1

[Stephan Leroux [:sleroux]]

Ah favicons.

This is an interesting problem. Unlike Top Sites, when dealing with Logins, we only have the website URL - not a Site object from the database. These URLS don't live in the History database. I feel like our current favicon/site abstraction doesn't handle this case well yet and might require some refactoring of how we do favicons as a whole unless I'm missing something?

Comment 2

[Richard Newman [:rnewman]]

Yeah, favicons are a mess :)

But don't worry about Site; it's just a wrapper for a history item, and not a good one (e.g., it doesn't understand multiple icon sizes, it's reused as a Top Sites tile, etc.).

You can do one of two things:

1. Try to add a faviconID to logins, just as we do for bookmarks. You'll need to know the icon in browser.db when you write the login. This has huge referential issues: if we delete favicons or lose browser.db, the favicon IDs will be invalid.

2. Look for the URL in history when you query.


Both of these are more complicated than for plain history, because of course the logins table is in a completely different database, and so you have referential problems and can't just do a join.

My recommendation is to load icons by history URL:

1. Fetch logins from logins.db.
2. Grab their URLs in memory.
3. Fetch icons by URL from browser.db, yielding an array of the same length.
4. Join in-memory and display.


On Android we'd do 3-4 asynchronously, but I'll settle for blocking display in this case!

Comment 3

[Stephan Leroux [:sleroux]]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1445

Adds favicons for login list and detail. Also includes an algorithm for finding the best fitting favicon for an image size.

Comment 4

[Stephan Leroux [:sleroux]]

Also resolves https://bugzilla.mozilla.org/show_bug.cgi?id=1239755.

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8708080 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1445

Comments on the PR.

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 8708080 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1445

Not a complete review, but should be enough to keep you busy!

Comment 7

[Stephan Leroux [:sleroux]]

As discussing in the PR this is going to get pushed back to 3.0. A temporary solution has been merged in with https://github.com/mozilla/firefox-ios/pull/1485 so we don't always see the default favicon.

Comment 8

[Richard Newman [:rnewman]]

Too much complexity.