Improve the TopSites WebExtension API
Categories
(WebExtensions :: General, task, P3)
Tracking
(firefox69 fixed)
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: mikedeboer, Assigned: mak)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 1 obsolete file)
For more info, see the design doc over at https://docs.google.com/document/d/11c-OTsscmArnqL02Kq1TUPKtkHl4avmC3pDdSJDC6JI/edit
There’s an existing topSites webextension API but not clear it has some things we need.
Comment hidden (obsolete) |
Assignee | ||
Comment 2•6 years ago
|
||
So, the existing topSites API is baked by NewTabUtils.getTopFrecentSites(), that is just returning a list of the most frecent urls. It is not returning the list shown on about:newtab, and in particular it only returns urls, no search engines. So I guess we'll need our own API, or to change the existing topSites API to also return the list from the new tab.
NewTabUtils.links.getLinks() returns what we want.
For the favicons problem, we could make the extensionprovider add an icon when it converts results, in the end it can use the engine icon for shortcuts and a page-icon: for other urls.
I'll try to look around for Shane to discuss the best path forward.
Assignee | ||
Comment 3•6 years ago
•
|
||
So I evaluated our options with Shane and Philipp, we are going to do the following:
- add a new option to topSites.get(), as an enum string, that allows to switch mode between "frequent" and "newtab" (names are made up for now, we also need a name for the option, maybe "mode" or "sortingMode" or "resultSet"?)
- the default for the API will remain the current result set, in the future the add-ons team will evaluate whether to change the default
- add a new property to the MostVisitedURL object, to indicate if it's a search (maybe just a type property) and again default to the current type (I suppose at the beginning the only 2 types will be "url" or "search", where url is the default).
- if possible, provide favicons when the includeFavicon option is true, if it's hard or slow we can leave the favicons out and document that "newtab" mode doesn't yet return favicons. We can add favicon urls in the UrlbarProviderExtension object, as I said in comment 2
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
Adds a "newtab" resultSet to the topSites API, so that we can get the actual
list of results shown on the newtab page. The old behavior (most frecent pages)
is retained as the default for now.
This may not be working completely on Android, but the topSites API situation
there is foggy already, the test is marked as a skip and part of the code is
using Places methods not available there. Should likely be handled elsewhere.
Assignee | ||
Comment 5•6 years ago
|
||
Adds includePinned and includeSearchShortcuts options to the topSites API, so
that it's possible to get a list of results like it's shown on the newtab page.
Both options default to false, so that the existing behavior is preserved.
The API gets disabled on Android, the API always depended on not available APIs
and the test is disabled on Android, so it'd be untested there.
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 9•6 years ago
|
||
This is primarily a webextension api enhancement, moving to our component.
Comment 10•6 years ago
|
||
bugherder |
Comment 11•6 years ago
|
||
Can you please provide some steps so we can manually verify this issue? If no manual testing is needed can you please mark it as "qe-verify- "
Assignee | ||
Comment 12•6 years ago
|
||
I don't think it needs manual QA, thanks.
Comment 13•6 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #5)
Adds includePinned and includeSearchShortcuts options to the topSites API, so
that it's possible to get a list of results like it's shown on the newtab page.
This doesn't seem to be working right: I'm not getting unvisited top sites listed on newtab (e.g. YouTube, Facebook, Reddit, Twitter). I haven't investigated very much yet.
Comment 14•6 years ago
|
||
Oh I see Ed said this in phabricator:
This should match up pretty close as is if we don't want to exactly exactly match
TopStoriesFeed's default sites for small history
But we do want that for the top sites experiment, don't we? e.g. users who just switched to Firefox should see the default suggested sites, same as new tab, no? Verdi?
Assignee | ||
Comment 15•6 years ago
•
|
||
(In reply to Drew Willcoxon :adw from comment #13)
This doesn't seem to be working right: I'm not getting unvisited top sites listed on newtab (e.g. YouTube, Facebook, Reddit, Twitter). I haven't investigated very much yet.
I didn't add it because I didn't want to surprise the user with stuff that he didn't visit/know about. The new tab page has more implications for which it wants to be filled out, and did their own investigation about it. The urlbar had its own way to provide "empty" results in the preloaded-top-sites feature, that has never been finished nor enabled (it's still behind a disabled pref). Considered that, the risk looked too big to just cargo-cult the empty-list from new tab page.
Comment 16•6 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #14)
Oh I see Ed said this in phabricator:
This should match up pretty close as is if we don't want to exactly exactly match TopStoriesFeed's default sites for small history
But we do want that for the top sites experiment, don't we? e.g. users who just switched to Firefox should see the default suggested sites, same as new tab, no? Verdi?
This is correct. The idea of the top sites feature is that it should always exactly match the top sites displayed in activity stream.
Comment 17•6 years ago
|
||
Tracked in mdn/sprints 1914 (in progress) for documentation purposes.
Comment 18•6 years ago
|
||
Added the following two values to the definition of topSites.get:
includePinned {{optional_inline}}
Boolean. Includes sites that the user has pinned to the Firefox new tab..
Defaults to false.
includeSearchShortcuts {{optional_inline}}
Boolean. Includes search shortcuts that appear on the Firefox new tab.
Defaults to false.
The release notes also have an entry:
The topSites.get() method now has new options available — includePinned and includeSearchShortcuts (bug 1547669).
Let me know if I missed anything.
Description
•