1617345 - Disabling top sites on new tab causes top sites not to be shown in the urlbar after restarting Firefox

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

[Tracking Requested - why for this release]:

If you hide top sites on new tab (set browser.newtabpage.activity-stream.feeds.topsites to false) and restart Firefox, focusing the urlbar doesn't show the top sites anymore. That's because this.activityStream.store.getState().TopSites.rows is an empty array here: https://searchfox.org/mozilla-central/rev/c1e3d3edd4a9b784971555dc74a5de23d768b2e1/browser/modules/AboutNewTab.jsm#115 Which makes total sense because top sites are disabled.

Ed, how might we fix this? We need to show the top sites in the urlbar view even when they're hidden on new tab. Setting the pref to true, calling AboutNewTab.getTopSites, and then resetting it to false actually works, but it's hacky, and it also causes the top sites to flicker on and off if the current tab is new tab.

Requesting tracking because we want to enable top sites in the urlbar on release on 74.

Comment 1

[Drew Willcoxon :adw]

Ed, please see comment 0.

Comment 2

[Brandon Watkins]

In the latest 74 beta update, I can't get the urlbar topsites dropdown to work even though I do have top sites enabled on the new tab page.

When I click the addressbar no dropdown is shown

Comment 3

[Marco Bonardo [:mak]]

could you please attach a log from about:support?

Comment 4

[Brandon Watkins]

Attachment: about:support log

about:support info attached

Comment 5

[Marco Bonardo [:mak]]

I wonder if there's a relation with privacy.usercontext.about_newtab_segregation.enabled, it doesn't show in your log but there's a
privacy.sanitize.pending: [{"id":"newtab-container","itemsToClear":[],"options":{}}]

Comment 6

[Brandon Watkins]

I tried setting that key to false (it was true) and restarting firefox, but I still don't see the top sites when clicking in the urlbar

It was working for me in 74b5 or 74b6 (I forget which one I was running before applying the 64b7 update)

Comment 7

[Brandon Watkins]

actually, it looks like that key automatically gets set back to True whenever I restart firefox

Comment 8

[Marco Bonardo [:mak]]

I think it's because you have these add-ons, but I'm not sure whether there's a relation, I was mostly guessing based on the log:

Name: Facebook Container
Version: 2.0.3
Enabled: true
ID: @contain-facebook

Name: Firefox Multi-Account Containers
Version: 6.2.1
Enabled: true
ID: @testpilot-containers

Maybe you could try Firefox in Safe Mode https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode and if it doesn't reproduce there, we'd have a hint that maybe the add-ons are related.

Comment 9

[Brandon Watkins]

I still see the issue after starting firefox in safe mode

Comment 10

[Brandon Watkins]

same thing happened on my other computer:

it was on 74b5 where topsites in urlbar was working

then updated to 74b6, still working

then updated to 74b7, no longer working

Comment 11

[Marco Bonardo [:mak]]

Oh wait, from beta 6 on the whole new design is disabled, it was supposed to stay enabled only in the initial beta versions. beta 5 is the last one with the new design, Firefox 75 beta will enable it again.
Beta 6 and 7 should have the old design, where you must click on the dropmarker to get a list of the most frequently visited pages.

Comment 12

[Ed Lee :Mardak]

I suppose AboutNewTab.getTopSites() could maybe return NewTabUtils.activityStreamLinks.getTopSites() (it's async) if it otherwise would return []. It's not quite the same as the new tab page's final top sites, but if the user has turned off top sites, the difference wouldn't be as noticeable anyway.

But if it's desired to have urlbar show the same sites as if new tab showed it, more complicated would be to have TopSitesFeed to keep running even when the user has turned off the section. This should "just" be creating a new pref exposed to about:preferences and having new tab page not render the section.

Comment 13

[Ed Lee :Mardak]

Although in the past, we had exactly the separate pref showTopSites and bugs like bug 1445769 were filed.

Comment 14

[Drew Willcoxon :adw]

Thanks Ed. Returning NewTabUtils.activityStreamLinks.getTopSites() sounds like a plan to me. I forgot that's what the top-sites WebExt API returned before we added the newtab option.

We wouldn't need to include pinned links, although I guess it's possible the user pinned some links before disabling top sites on new tab.

I'm not sure about search shortcuts.

Comment 15

[Drew Willcoxon :adw]

Or, if we're not going to try to show the new-tab top sites, we can just fall back to the old history popup we used to show.

Comment 16

[Drew Willcoxon :adw]

Attachment: Bug 1617345 - If top sites are disabled, show the user's most frecent sites instead.

In other words, for users who have disabled top sites, this restores the history popup we used to show when you clicked the dropmarker or pressed the down arrow key in an empty urlbar.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61de73dc1122
If top sites are disabled, show the user's most frecent sites instead. r=harry

Comment 18

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/61de73dc1122

Comment 19

[Marco Bonardo [:mak]]

Comment on attachment 9129032 [details]
Bug 1617345 - If top sites are disabled, show the user's most frecent sites instead.

Beta/Release Uplift Approval Request

• User impact if declined: We plan to run a pref-flip experiment in Firefox 74 with the new urlbar design, this is a necessary fix for that.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: With design update 1 enabled (set at least browser.urlbar.update1 and browser.urlbar.openViewOnFocus), set browser.newtabpage.activity-stream.feeds.topsites = false, restart firefox, check Top Sites in urlbar still show a list of visited pages
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects disabled feature
• String changes made/needed:

Comment 20

[Daniel Kolsis]

Verified on Nightly 75.0a1 (2020-02-27) - the Top sites are now shown when clicking on the address bar. Waiting for uplift into Beta.

Comment 21

[Pascal Chevrel:pascalc]

Comment on attachment 9129032 [details]
Bug 1617345 - If top sites are disabled, show the user's most frecent sites instead.

Needed for a 74 experiment, uplift approved for 74 beta 9, thanks.

Comment 22

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3a4503c795d7

Comment 23

[Harry Twyford [:harry]]

Verdi has asked that we show Top Sites in the Urlbar regardless of whether they are enabled on about:newtab, rather than the frecent sites re-enabled in this patch. This will entail splitting the single topSites pref into one pref that enables them, accessible only in about:config; and one that shows them on about:newtab, exposed to about:preferences. This is the approach Ed pointed out in comment 12. This means essentially undoing bug 1445769. Ed, before we proceed, do you see any issues with this approach?

Comment 24

[Ed Lee :Mardak]

Looks like this was fixed with bug 1634279. The original concern with the separate pref was doing unnecessary work in the background, but with multiple consumers even with the new tab page not showing top sites makes it reasonable to have a separate control.

Comment 25

[Brindusa Tot[:brindusat]]

This is on Release from some time now. Remove the qe-verify+ flag.