1317536 - Top Sites is showing links to specific pages of a domain instead of just the domain's home page

Status: RESOLVED FIXED

(Firefox for iOS :: Home screen, defect, P1)

Description

[Farhan Patel [:farhan]]

On a fresh install

1. Go to http://digg.com/source/techcrunch.com
2. Click a few articles that take you to techcrunch. 
3. A top site tile should appear for techcrunch

Expected: The techcrunch tile should take you to the homepage

Actual: It takes you to one of the articles you visited.

Comment 1

[Richard Newman [:rnewman]]

(Obligatory back-pointer to Bug 1184582.)

Comment 2

[Aaron Train [:aaronmt]]

I find this bug rather annoying.

Comment 3

[Stephan Leroux [:sleroux]]

See if this also is happening on our AS panel top sites.

Comment 4

[Farhan Patel [:farhan]]

Attachment: Pull Request

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 8822566 [details] [review]
Pull Request

Worth noting that this behavior will be less desirable in certain situations (e.g., I have https://people-mozilla.org/~bnicholson/test/ as a top site; https://people-mozilla.org is useless on its own), but I expect that as a sum, this will be an overall improvement.

Left one comment about components being stripped using domainURL -- I don't think we'll want to remove the www/mobile/m prefix.

Comment 6

[Richard Newman [:rnewman]]

Isn't this the exact opposite of Bug 1231515, desktop's behavior, and the end result we worked out in Bug 1184582?

What we wanted to do was roughly what desktop does:

- If there's a clear winner, use that: if you go to the same TechCrunch article fifty times, because you love it, then we should take you there rather than techcrunch.com.
- If there isn't, use 'greatest common sub-path': perhaps you always go to amazon.com/mp3. Or say I visit pages under https://reddit.com/r/cats a lot, I'll end up with a 'reddit' tile… and it should take me to /r/cats, not /.
- Otherwise, use the domain.

We didn't get all the way there, but it's pretty likely that /r/cats will 'win' the tile, and so *mostly* Fennec's Top Sites does what you expect. If it doesn't, it will at least take you somewhere you've actually been.

Where that falls down is Gmail and the like, which have a broad spread of URLs, and each one is useless… but the best URL for Gmail is https://mail.google.com/mail, so we're back to greatest common sub-path.

If, on the other hand, you always go to bbc.com/news, this PR will cause your tile to always take you to bbc.com/. That's a regression -- you're introducing predictability, but it's a predictably bad result!

My opinion: if you want to fix this, do it right: calculate a 'winner' URL for a domain as part of the Top Sites computation, and use that for the tile. This PR alone isn't worth the churn in the user experience.

Comment 7

[bbell]

Top Sites shouldn't only support links to domain home pages. Top Sites should have a long (ish) view of a users activity and present to them their "for sure top utilized" sites. It should avoid recently interacted content since that's what highlights is meant to address. If a user is in fact a repeat user of a specific article and they have been for a long while then that would definitely be a Top Site. 

The desktop team has worked a long while on Top Sites, while its still not completely perfect it has been getting pretty good, can we look into evaluating the system the Desktop Team is using and mimic that or simple use that same system for mobile?

Comment 9

[Farhan Patel [:farhan]]

I spent some time understand the SQL queries. Something weird is happening indeed. 


I have 3 different history items for google.ca.

1 - A search result page with one visit. (localvisit)
2 - A search result page with 3 visits. where one is a remoteVisit
3 - The google.ca homepage with 8 localvisits. 

So naturally, the TopSite url should be #3 but actually number 2 wins!? weird. 

Looking at the comments in the code and in the respective bugs localvisits are suppose to be weighted heavier but something funky is going on. 

I've been verifying this by opening my DB in a sqlite browser.

Comment 10

[Farhan Patel [:farhan]]

Attachment: Pull Request

Comment 11

[Richard Newman [:rnewman]]

Comment on attachment 8855356 [details] [review]
Pull Request

I left a pointer to Bug 1311150 Comment 2, which should explain why this isn't a fix.

Comment 12

[Farhan Patel [:farhan]]

Comment on attachment 8855356 [details] [review]
Pull Request

I've added a note about the GROUP BY usage. Ready for re-review

Comment 13

[Farhan Patel [:farhan]]

Comment on attachment 8855356 [details] [review]
Pull Request

was marked as review on GH

Comment 14

[Farhan Patel [:farhan]]

master https://github.com/mozilla-mobile/firefox-ios/commit/748e137dfd5b020b56fc481b25e0d2366acb3df2