Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Commit

Permalink
Fix Bug 1482629 - Compare search topsites against search engine _inte…
Browse files Browse the repository at this point in the history 
…rnalAliases (#4326)
  • Loading branch information
@piatra @Mardak
piatra authored and Mardak committed Aug 13, 2018
1 parent 93c6b1d commit 70111e2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 27 deletions.
30 changes: 20 additions & 10 deletions lib/SearchShortcuts.jsm
View file
Expand Up @@ -3,24 +3,26 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";

ChromeUtils.import("resource://gre/modules/Services.jsm");

// List of sites we match against Topsites in order to identify sites
// that should be converted to search Topsites
const SEARCH_SHORTCUTS = [
{keyword: "@amazon", shortURL: "amazon", url: "https://amazon.com", searchIdentifier: /^amazon/},
{keyword: "@\u767E\u5EA6", shortURL: "baidu", url: "https://baidu.com", searchIdentifier: /^baidu/},
{keyword: "@google", shortURL: "google", url: "https://google.com", searchIdentifier: /^google/},
{keyword: "@\u044F\u043D\u0434\u0435\u043A\u0441", shortURL: "yandex", url: "https://yandex.com", searchIdentifier: /^yandex/}
{keyword: "@amazon", shortURL: "amazon", url: "https://amazon.com"},
{keyword: "@\u767E\u5EA6", shortURL: "baidu", url: "https://baidu.com"},
{keyword: "@google", shortURL: "google", url: "https://google.com"},
{keyword: "@\u044F\u043D\u0434\u0435\u043A\u0441", shortURL: "yandex", url: "https://yandex.com"}
];
this.SEARCH_SHORTCUTS = SEARCH_SHORTCUTS;

// These can be added via the editor but will not be added organically
this.CUSTOM_SEARCH_SHORTCUTS = [
...SEARCH_SHORTCUTS,
{keyword: "@bing", shortURL: "bing", url: "https://bing.com", searchIdentifier: /^bing/},
{keyword: "@duckduckgo", shortURL: "duckduckgo", url: "https://duckduckgo.com", searchIdentifier: /^ddg/},
{keyword: "@ebay", shortURL: "ebay", url: "https://ebay.com", searchIdentifier: /^ebay/},
{keyword: "@twitter", shortURL: "twitter", url: "https://twitter.com", searchIdentifier: /^twitter/},
{keyword: "@wikipedia", shortURL: "wikipedia", url: "https://wikipedia.org", searchIdentifier: /^wikipedia/}
{keyword: "@bing", shortURL: "bing", url: "https://bing.com"},
{keyword: "@duckduckgo", shortURL: "duckduckgo", url: "https://duckduckgo.com"},
{keyword: "@ebay", shortURL: "ebay", url: "https://ebay.com"},
{keyword: "@twitter", shortURL: "twitter", url: "https://twitter.com"},
{keyword: "@wikipedia", shortURL: "wikipedia", url: "https://wikipedia.org"}
];

// Note: you must add the activity stream branch to the beginning of this if using outside activity stream
Expand All @@ -33,5 +35,13 @@ function getSearchProvider(candidateShortURL) {
}
this.getSearchProvider = getSearchProvider;

const EXPORTED_SYMBOLS = ["getSearchProvider", "SEARCH_SHORTCUTS", "CUSTOM_SEARCH_SHORTCUTS", "SEARCH_SHORTCUTS_EXPERIMENT",
// Check topsite against predefined list of valid search engines
// https://searchfox.org/mozilla-central/rev/ca869724246f4230b272ed1c8b9944596e80d920/toolkit/components/search/nsSearchService.js#939
function checkHasSearchEngine(keyword) {
return Services.search.getDefaultEngines()
.find(e => e.wrappedJSObject._internalAliases.includes(keyword));
}
this.checkHasSearchEngine = checkHasSearchEngine;

const EXPORTED_SYMBOLS = ["checkHasSearchEngine", "getSearchProvider", "SEARCH_SHORTCUTS", "CUSTOM_SEARCH_SHORTCUTS", "SEARCH_SHORTCUTS_EXPERIMENT",
"SEARCH_SHORTCUTS_SEARCH_ENGINES_PREF", "SEARCH_SHORTCUTS_HAVE_PINNED_PREF"];
13 changes: 6 additions & 7 deletions lib/TopSitesFeed.jsm
View file
Expand Up @@ -16,6 +16,7 @@ const {
SEARCH_SHORTCUTS_EXPERIMENT,
SEARCH_SHORTCUTS_SEARCH_ENGINES_PREF,
SEARCH_SHORTCUTS_HAVE_PINNED_PREF,
checkHasSearchEngine,
getSearchProvider
} = ChromeUtils.import("resource://activity-stream/lib/SearchShortcuts.jsm", {});

Expand Down Expand Up @@ -190,7 +191,7 @@ this.TopSitesFeed = class TopSitesFeed {
!pinnedSites.find(s => s && s.hostname === shortcut.shortURL) &&
!prevInsertedShortcuts.includes(shortcut.shortURL) &&
nextAvailable > -1 &&
Services.search.getEngines().find(e => e.identifier && e.identifier.match(shortcut.searchIdentifier))
checkHasSearchEngine(shortcut.keyword)
) {
const site = this.topSiteToSearchTopSite({url: shortcut.url});
this._pinSiteAt(site, nextAvailable);
Expand Down Expand Up @@ -376,11 +377,9 @@ this.TopSitesFeed = class TopSitesFeed {
// Populate the state with available search shortcuts
await new Promise(resolve => Services.search.init(resolve));
const searchShortcuts = Services.search.getDefaultEngines().reduce((result, engine) => {
if (engine.identifier) {
const shortcut = CUSTOM_SEARCH_SHORTCUTS.find(s => engine.identifier.match(s.searchIdentifier));
if (shortcut) {
result.push(this._tippyTopProvider.processSite({...shortcut}));
}
const shortcut = CUSTOM_SEARCH_SHORTCUTS.find(s => engine.wrappedJSObject._internalAliases.includes(s.keyword));
if (shortcut) {
result.push(this._tippyTopProvider.processSite({...shortcut}));
}
return result;
}, []);
Expand All @@ -392,7 +391,7 @@ this.TopSitesFeed = class TopSitesFeed {

topSiteToSearchTopSite(site) {
const searchProvider = getSearchProvider(shortURL(site));
if (!searchProvider) {
if (!searchProvider || !checkHasSearchEngine(searchProvider.keyword)) {
return site;
}
return {
Expand Down
15 changes: 5 additions & 10 deletions test/unit/lib/TopSitesFeed.test.js
View file
Expand Up @@ -1182,14 +1182,11 @@ describe("Top Sites Feed", () => {
feed.store.state.Prefs.values[SEARCH_SHORTCUTS_EXPERIMENT_PREF] = true;
feed.store.state.Prefs.values[SEARCH_SHORTCUTS_SEARCH_ENGINES_PREF] = "google,amazon";
feed.store.state.Prefs.values[SEARCH_SHORTCUTS_HAVE_PINNED_PREF] = "";
global.Services.search.getEngines = () => [
{identifier: "google"},
{identifier: "amazon"}
];
global.Services.search.getDefaultEngines = () => [
{identifier: "google"},
{identifier: "amazon"}
const searchEngines = [
{wrappedJSObject: {_internalAliases: ["@google"]}},
{wrappedJSObject: {_internalAliases: ["@amazon"]}}
];
global.Services.search.getDefaultEngines = () => searchEngines;
fakeNewTabUtils.pinnedLinks.pin = sinon.stub().callsFake((site, index) => {
fakeNewTabUtils.pinnedLinks.links[index] = site;
});
Expand Down Expand Up @@ -1295,12 +1292,10 @@ describe("Top Sites Feed", () => {
data: {
searchShortcuts: [{
keyword: "@google",
searchIdentifier: /^google/,
shortURL: "google",
url: "https://google.com"
}, {
keyword: "@amazon",
searchIdentifier: /^amazon/,
shortURL: "amazon",
url: "https://amazon.com"
}]
Expand Down Expand Up @@ -1355,7 +1350,7 @@ describe("Top Sites Feed", () => {

it("should not pin a shortcut if the corresponding search engine is not available", async () => {
// Make Amazon search engine unavailable
global.Services.search.getEngines = () => [{identifier: "google"}];
global.Services.search.getDefaultEngines = () => [{wrappedJSObject: {_internalAliases: ["@google"]}}];
fakeNewTabUtils.pinnedLinks.links.fill(null);
await feed._maybeInsertSearchShortcuts(fakeNewTabUtils.pinnedLinks.links);
assert.notOk(fakeNewTabUtils.pinnedLinks.links.find(s => s && s.url === "https://amazon.com"));
Expand Down

0 comments on commit 70111e2

Please sign in to comment.