Closed Bug 1617345 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Address Bar, defect, P1)

defect
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 75
Iteration:
75.2 - Feb 24 - Mar 8
Tracking Status
firefox74 + fixed
firefox75 --- verified

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(2 files)

[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.

Ed, please see comment 0.

Flags: needinfo?(edilee)

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

could you please attach a log from about:support?

Flags: needinfo?(bwat47)
Attached file about:support log (deleted) —

about:support info attached

Flags: needinfo?(bwat47)

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":{}}]

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)

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

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.

I still see the issue after starting firefox in safe mode

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

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.

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.

Flags: needinfo?(edilee)

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

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.

Assignee: nobody → adw
Status: NEW → ASSIGNED
Iteration: --- → 75.2 - Feb 24 - Mar 8
Points: --- → 2

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.

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.

Pushed by dwillcoxon@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/61de73dc1122 If top sites are disabled, show the user's most frecent sites instead. r=harry
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75

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:
Attachment #9129032 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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 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.

Attachment #9129032 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1623666

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?

Flags: needinfo?(edilee)

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.

Flags: needinfo?(edilee)

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

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Regressions: 1654676
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: