1257592 - Pages loaded with Top Sites open don't refresh Top Sites

Status: RESOLVED WONTFIX

(Firefox for iOS :: Home screen, defect)

Description

[Brian Nicholson (:bnicholson)]

STR:
1) Go to https://people.mozilla.org/~bnicholson/test/delayload.html.
2) Within 5 seconds, tap the URL bar.
3) Wait 5 seconds.

nytimes.com doesn't appear in Top Sites until I leave and return to the panel.

For a more real-world scenario, replace the URL in step 1 with any site that takes a few seconds to load.

We already update Top Sites when for sync complete (and other) notifications, so it would be straightforward to add the location change notification to this list.

Comment 1

[Brian Nicholson (:bnicholson)]

BTW, I'm not completely sold that we want to do this -- I think it would be valid to consider the Top Sites panel "frozen" while the user is looking at it. It could be annoying if the site of sites changes out from under you while you're using the UI.

But I do want to push for consistency: as mentioned above, we *do* already do live updates on sync completions, so if we like that behavior, it makes sense to do it here, too.

I'd consider this fixed for one of the following:
1) Refreshing Top Sites on location change as mentioned above.
2) *Not* live refreshing on sync changes.

We refresh on sync now because we want to handle the case where the user goes to Settings > Sync and returns to their Top Sites, so if we like option 2, we could change the condition to refresh only if the panel isn't visible.

Comment 2

[Robin Andersen [:tecgirl]]

I think it works as-is. If I'm loading a site, why would I expect to see it show up automatically if I hit topsites while loading it?

Comment 3

[Brian Nicholson (:bnicholson)]

See comment 1. If we don't want sites to refresh Top Sites while we're looking at it, we should change our current Sync behavior, right? As it stands now, a Sync can cause all of the top sites to change in front of the user.

Comment 4

[Robin Andersen [:tecgirl]]

I guess I'm not understanding the problem. Yes, a sync will cause all the top sites to change, but are you suggesting that there's an expectation to see the live sync when you have just visited a site?

Comment 5

[Richard Newman [:rnewman]]

Here's the way I see it.

We don't, as a rule, change Top Sites while you're viewing it: the situation in Comment 0 is a very artificial one.

Not only is it uncommon for a page to load significant new domains while you're hawkishly watching Top Sites, but for established users a single page load isn't going to affect Top Sites anyway!

Sync is an exception to this rule because new users set up Sync, back out of Settings, and don't see their downloaded data until they create a new tab. That's jarring.

It's not enough to only refresh in the background; perhaps the sync finishes 100msec after they return to Top Sites!

Consistency is only an end goal if (a) the user notices it, and (b) the user has a mental model that demands it. I don't think either is true in this case, and the cost of refreshing top sites on every page load is significant, so we shouldn't do it.

Comment 6

[Brian Nicholson (:bnicholson)]

(In reply to Robin Andersen [:tecgirl] from comment #4)
> I guess I'm not understanding the problem. Yes, a sync will cause all the
> top sites to change, but are you suggesting that there's an expectation to
> see the live sync when you have just visited a site?

My question is: do we want top sites to be updated if the history changes while they're visible?

An incoming sync from other devices causes new history to be added, potentially changing the set of top sites. A page that finishes loading in the background also adds a new history entry, also potentially changing the set of top sites. In the first case (sync), we update the top sites while the user is looking at them when the history changes; in the second case (pages loaded in the background), we don't. These actions both have the same effect on history -- new history items being added -- so my assertion is that Top Sites should handle them the same.

(In reply to Richard Newman [:rnewman] from comment #5)
> We don't, as a rule, change Top Sites while you're viewing it: the situation
> in Comment 0 is a very artificial one.

> Not only is it uncommon for a page to load significant new domains while
> you're hawkishly watching Top Sites

Not artificial/uncommon at all -- as mentioned in comment 0, you can reproduce this by tapping the URL bar any time the site hasn't started to load yet. I see this a lot, whether it's due to a poor connection on my end or a slow site.

> but for established users a single page load isn't going to affect
> Top Sites anyway!

For established users, sure -- and that just means refreshing after these cases will have no effect, which is fine. For newer users, Top Sites will change frequently with each page load.

> the cost of refreshing top sites on every page load is significant

For this bug, I'm suggesting we simply add OnLocationChange to the observed events at [1] (that is, assuming we want to go with option 1 in comment 1). I don't think the cost is significant; we will only do the refresh when it matters and when Top Sites is visible.

If by "refreshing top sites on every page load" you mean updating the DB for each location change even when Top Sites isn't visible, that's a different bug altogether. That said, I still think that'd be an important change to consider: see [2]!

[1] https://github.com/mozilla/firefox-ios/blob/565568b1f9a9517d1747fee5a6654d7ea82e3ff8/Client/Frontend/Home/TopSitesPanel.swift#L119
[2] https://github.com/mozilla/firefox-ios/pull/1642#issuecomment-199462765