1329955 - Consolidate new tab page code into browser/components/newtab

Status: NEW

(Firefox :: New Tab Page, defect, P3)

Description

[:Gijs (out for now; he/him)]

Besides browser/components/newtab, there's toolkit/modules/NewTabUtils.jsm (which I'm not sure we can move without looking in more detail), browser/base/content/newtab/, browser/base/content/tests/newtab/, and browser/modules/AboutNewTab.jsm . I think all of the code should live under browser/components/newtab/ . It can grow a content/ dir and we can move the tests.

Comment 1

[Ed Lee :Mardak]

From mconley's bug 1386445 comment 6:
> I'd even go so far as to say perhaps AboutNewTab.jsm and aboutNewTabService should be combined at some point - but that's probably too big a refactor for our purposes here.

The patch for that bug ended up calling AboutNewTab.init from aboutNewTabService constructor and the code for AboutNewTab could very well fit in the service as it's quite small and will probably get even smaller when activity stream is on by default and replaces tiles without an option to turn off activity stream:
https://searchfox.org/mozilla-central/source/browser/modules/AboutNewTab.jsm

Comment 2

[Tim Spurway [:tspurway]]

Ed can this be closed now?

Comment 3

[Ed Lee :Mardak]

The files are still spread out. Probably makes sense to clean up when activity stream is the new tab with no way to revert to tiles.

Comment 4

[Ed Lee :Mardak]

Bug 1433133 removed browser/base/content/newtab and browser/base/content/test/newtab
Bug 1355166 removed some files from browser/components/newtab

There's been various cleanups of NewTabUtils along the way, e.g., bug 1241390

Separately, there's discussion in bug 1440421 to make mozilla-central the primary repository for activity stream (with github effectively a clone/mirror).

I don't think there's a bug filed yet, but sounds like the direction from Austin all hands was to move activity stream to browser/components to stop using the legacy extension api.

At least these non-activity-stream newtab files are often touched with activity stream changes: aboutNewTabService, NewTabUtils.

So at least right now, looks like the consolidation would be of toolkit/modules/NewTabUtils browser/modules/AboutNewTab browser/extensions/activity-stream and browser/components/newtab. Where potentially everything consolidates somehow to the last one?

Comment 5

[Ed Lee :Mardak]

The current plan for bug 1447130 is to move browser/extensions/activity-stream into browser/components/newtab (keeping newtab name for now, but potentially renaming to browser/components/activity-stream…)

As part of that meta, bug 1474410 will probably move AboutNewTab.jsm from browser/modules to components.

With activity-stream handling about:home, felipe removed browser/base/content/abouthome in bug 1409054. We could maybe consolidate various browser/base/content/test/about/browser_aboutHome_* tests to browser/components... or just leave them there. ;)

Not covered in the activity-stream -> component meta bug is NewTabUtils, which might be the only thing left to consolidate?

Comment 6

[Ed Lee :Mardak]

AboutNewTab did not end up moving as part of the component changes, so that jsm and NewTabUtils both still need to move. There's some work involved more than just moving as when the files end up in browser/components/newtab directory, activity-stream has more eslint rules checked (and erroring right now).

Comment 7

[Ed Lee :Mardak]

A current search for uses of NewTabUtils shows the only non-browser non-test usage is for extensions:

https://searchfox.org/mozilla-central/search?q=NewTabUtils.jsm
toolkit/components/extensions/parent/ext-topSites.js

There's a couple test usages:

• extensions for above
• thumbnails to populateCache to trigger thumbnails

The latter probably wants some reworking to just trigger thumbnail fetching some other way and the populateCache and related API could probably just be deleted.