From 9e5e02bfee7c4ff119d0a409e80fba56b2ba07f5 Mon Sep 17 00:00:00 2001 From: Michael Kaply Date: Fri, 29 Apr 2016 17:08:55 +0200 Subject: [PATCH] Bug 1181645 - Default invalid or empty purpose in getSubmission to searchbar, r=florian. a=ritu MozReview-Commit-ID: GdgAjVzzkro --- browser/components/search/test/browser_bing.js | 4 ++-- .../search/test/browser_bing_behavior.js | 2 +- .../components/search/test/browser_yahoo.js | 18 ++++++++++++++++-- .../search/test/browser_yahoo_behavior.js | 2 +- browser/locales/en-US/searchplugins/google.xml | 2 +- toolkit/components/search/nsSearchService.js | 9 ++++----- .../search/tests/xpcshell/data/engine.xml | 4 ++-- .../search/tests/xpcshell/test_purpose.js | 9 +++++---- 8 files changed, 32 insertions(+), 18 deletions(-) diff --git a/browser/components/search/test/browser_bing.js b/browser/components/search/test/browser_bing.js index 6327036e11d..592f5cfddc7 100644 --- a/browser/components/search/test/browser_bing.js +++ b/browser/components/search/test/browser_bing.js @@ -18,7 +18,7 @@ function test() { // Test search URLs (including purposes). url = engine.getSubmission("foo").uri.spec; - is(url, base, "Check search URL for 'foo'"); + is(url, base + "&form=MOZSBR", "Check search URL for 'foo'"); url = engine.getSubmission("foo", null, "contextmenu").uri.spec; is(url, base + "&form=MOZCON", "Check context menu search URL for 'foo'"); url = engine.getSubmission("foo", null, "keyword").uri.spec; @@ -39,7 +39,7 @@ function test() { name: "Bing", alias: null, description: "Bing. Search by Microsoft.", - searchForm: "https://www.bing.com/search?q=&pc=MOZI", + searchForm: "https://www.bing.com/search?q=&pc=MOZI&form=MOZSBR", hidden: false, wrappedJSObject: { queryCharset: "UTF-8", diff --git a/browser/components/search/test/browser_bing_behavior.js b/browser/components/search/test/browser_bing_behavior.js index 1ea6a9d401b..bc9b187ec0e 100644 --- a/browser/components/search/test/browser_bing_behavior.js +++ b/browser/components/search/test/browser_bing_behavior.js @@ -23,7 +23,7 @@ function test() { // Test search URLs (including purposes). url = engine.getSubmission("foo").uri.spec; - is(url, base, "Check search URL for 'foo'"); + is(url, base + "&form=MOZSBR", "Check search URL for 'foo'"); waitForExplicitFinish(); diff --git a/browser/components/search/test/browser_yahoo.js b/browser/components/search/test/browser_yahoo.js index 8d5ec501b49..f45b47d0cc9 100644 --- a/browser/components/search/test/browser_yahoo.js +++ b/browser/components/search/test/browser_yahoo.js @@ -18,7 +18,21 @@ function test() { // Test search URLs (including purposes). url = engine.getSubmission("foo").uri.spec; - is(url, base, "Check search URL for 'foo'"); + is(url, base + "&hsimp=yhs-001", "Check search URL for 'foo'"); + url = engine.getSubmission("foo", null, "searchbar").uri.spec; + is(url, base + "&hsimp=yhs-001", "Check search bar search URL for 'foo'"); + url = engine.getSubmission("foo", null, "keyword").uri.spec; + is(url, base + "&hsimp=yhs-002", "Check keyword search URL for 'foo'"); + url = engine.getSubmission("foo", null, "homepage").uri.spec; + is(url, base + "&hsimp=yhs-003", "Check homepage search URL for 'foo'"); + url = engine.getSubmission("foo", null, "newtab").uri.spec; + is(url, base + "&hsimp=yhs-004", "Check newtab search URL for 'foo'"); + url = engine.getSubmission("foo", null, "contextmenu").uri.spec; + is(url, base + "&hsimp=yhs-005", "Check context menu search URL for 'foo'"); + url = engine.getSubmission("foo", null, "system").uri.spec; + is(url, base + "&hsimp=yhs-007", "Check system search URL for 'foo'"); + url = engine.getSubmission("foo", null, "invalid").uri.spec; + is(url, base + "&hsimp=yhs-001", "Check invalid URL for 'foo'"); // Check search suggestion URL. url = engine.getSubmission("foo", "application/x-suggestions+json").uri.spec; @@ -29,7 +43,7 @@ function test() { name: "Yahoo", alias: null, description: "Yahoo Search", - searchForm: "https://search.yahoo.com/yhs/search?p=&ei=UTF-8&hspart=mozilla", + searchForm: "https://search.yahoo.com/yhs/search?p=&ei=UTF-8&hspart=mozilla&hsimp=yhs-001", hidden: false, wrappedJSObject: { queryCharset: "UTF-8", diff --git a/browser/components/search/test/browser_yahoo_behavior.js b/browser/components/search/test/browser_yahoo_behavior.js index f8206df8f5f..5b2d61422d2 100644 --- a/browser/components/search/test/browser_yahoo_behavior.js +++ b/browser/components/search/test/browser_yahoo_behavior.js @@ -23,7 +23,7 @@ function test() { // Test search URLs (including purposes). url = engine.getSubmission("foo").uri.spec; - is(url, base, "Check search URL for 'foo'"); + is(url, base + "&hsimp=yhs-001", "Check search URL for 'foo'"); waitForExplicitFinish(); diff --git a/browser/locales/en-US/searchplugins/google.xml b/browser/locales/en-US/searchplugins/google.xml index 742bdcf588e..0642d56f94c 100644 --- a/browser/locales/en-US/searchplugins/google.xml +++ b/browser/locales/en-US/searchplugins/google.xml @@ -12,7 +12,7 @@ - + diff --git a/toolkit/components/search/nsSearchService.js b/toolkit/components/search/nsSearchService.js index cfc62b9e4f3..858232f0067 100644 --- a/toolkit/components/search/nsSearchService.js +++ b/toolkit/components/search/nsSearchService.js @@ -1140,12 +1140,11 @@ EngineURL.prototype = { getSubmission: function SRCH_EURL_getSubmission(aSearchTerms, aEngine, aPurpose) { var url = ParamSubstitution(this.template, aSearchTerms, aEngine); - // Default to an empty string if the purpose is not provided so that default purpose params - // (purpose="") work consistently rather than having to define "null" and "" purposes. - var purpose = aPurpose || ""; + // Default to searchbar if the purpose is not provided + var purpose = aPurpose || "searchbar"; - // If the 'system' purpose isn't defined in the plugin, fallback to 'searchbar'. - if (purpose == "system" && !this.params.some(p => p.purpose == "system")) + // If a particular purpose isn't defined in the plugin, fallback to 'searchbar'. + if (!this.params.some(p => p.purpose !== undefined && p.purpose == purpose)) purpose = "searchbar"; // Create an application/x-www-form-urlencoded representation of our params diff --git a/toolkit/components/search/tests/xpcshell/data/engine.xml b/toolkit/components/search/tests/xpcshell/data/engine.xml index 699f952eb23..e7af1d9e9ed 100644 --- a/toolkit/components/search/tests/xpcshell/data/engine.xml +++ b/toolkit/components/search/tests/xpcshell/data/engine.xml @@ -16,8 +16,8 @@ - - + + diff --git a/toolkit/components/search/tests/xpcshell/test_purpose.js b/toolkit/components/search/tests/xpcshell/test_purpose.js index c49052d59bc..46465e0a3c8 100644 --- a/toolkit/components/search/tests/xpcshell/test_purpose.js +++ b/toolkit/components/search/tests/xpcshell/test_purpose.js @@ -43,12 +43,13 @@ add_task(function* test_purpose() { // Tests for a param that varies with a purpose but has a default value. base = "http://www.google.com/search?q=foo"; - check_submission("&channel=none", "foo", "application/x-moz-default-purpose"); - check_submission("&channel=none", "foo", "application/x-moz-default-purpose", null); - check_submission("&channel=none", "foo", "application/x-moz-default-purpose", ""); + check_submission("&channel=ffsb", "foo", "application/x-moz-default-purpose"); + check_submission("&channel=ffsb", "foo", "application/x-moz-default-purpose", null); + check_submission("&channel=ffsb", "foo", "application/x-moz-default-purpose", ""); check_submission("&channel=rcs", "foo", "application/x-moz-default-purpose", "contextmenu"); check_submission("&channel=fflb", "foo", "application/x-moz-default-purpose", "keyword"); - check_submission("", "foo", "application/x-moz-default-purpose", "invalid"); + check_submission("&channel=ffsb", "foo", "application/x-moz-default-purpose", "searchbar"); + check_submission("&channel=ffsb", "foo", "application/x-moz-default-purpose", "invalid"); // Tests for a purpose on the search form (ie. empty query). engine = Services.search.getEngineByName("engine-rel-searchform-purpose");