1195683 - Restore "fr" preferences for non-US Yahoo! searchplugins

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[dynamis (Tomoya ASAI)]

Bug 1119560 (landed for Firefox 38) have removed all browser.search.param.yahoo-fr* but Yahoo Japan search plug-in still need browser.search.param.yahoo-fr-ja pref.

Please take back our pref:
  pref("browser.search.param.yahoo-fr-ja", "mozff");

Search plugin for Yahoo! Japan needs this pref to send affiliate code:
http://hg.mozilla.org/releases/l10n/mozilla-release/ja/file/tip/browser/searchplugins/yahoo-jp.xml

note:
Search url after Firefox 38 without the pref:
http://search.yahoo.co.jp/search?p=test&ei=UTF-8
Search url until Firefox 37 with the pref:
http://search.yahoo.co.jp/search?p=test&ei=UTF-8&fr=mozff

Comment 1

[Makoto Kato [:m_kato]]

[Tracking Requested - why for this release]:
This is affiliate code issue for Yahoo! Japan.  We should fix this on release channel.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

I mentioned this in our meeting just now. Sounds like we should enable the pref. Would we also do that for ESR?

Comment 3

[Kev Needham [:kev]]

I'm writing a patch now, and yes. Most changes in Bug 1119560 need to be reversed. Will be submitting and asking for immediate uplift, and a ride-along if we do a point release.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

We will take it for esr too.
Kev, we might do a 40.0.3 for GNU/Linux only but we could take it as other systems will be built too.

Comment 5

[Kev Needham [:kev]]

Attachment: Patch to restore fr codes for use by Yahoo Search Plugins on Desktop

Comment 6

[Kev Needham [:kev]]

Dynamis, this should address. look good?

Comment 7

[dynamis (Tomoya ASAI)]

Yahoo! Japan (and Yahoo! auction) search plug-in require browser.search.param.yahoo-fr-ja=mozff
http://hg.mozilla.org/l10n-central/ja/file/tip/browser/searchplugins/yahoo-jp.xml
http://hg.mozilla.org/l10n-central/ja/file/tip/browser/searchplugins/yahoo-jp-auctions.xml
# <MozParam name="fr" condition="pref" pref="yahoo-fr-ja" />

I think (not 100% sure but believe) we need not browser.search.param.yahoo-fr param for Yahoo! Japan. Yahoo Japan search plug-ins only check yahoo-fr-ja param (see above files). When we changed the parameter last time, we don't touch yahoo-fr, we only updated yahoo-fr-ja value (previously yahoo-fr-cjkt):
https://bugzilla.mozilla.org/show_bug.cgi?id=534871

In addition to this, we may need to update search.json in user profile explicitly. Even if we add browser.search.param.yahoo-fr-ja=mozff pref with about:config, search url will not include fr param untill we update search.json file.

Firefox currently don't use *.xml search plug-ins, we use json cache version of them. The json file is created with nsSearchService.js but it will drop <MozParam name="fr" condition="pref" pref="yahoo-fr-ja" /> part if browser.search.param.yahoo-fr-ja pref is not exist when we create/update search.json.

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#1406
        } else if (param.condition == "pref") {
          let value = getMozParamPref(param.pref);
          this.addParam(param.name, value);
        }
https://dxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#3170
  addParam: function SRCH_ENG_addParam(aName, aValue, aResponseType) {
    if (!aName || (aValue == null))
      FAIL("missing name or value for nsISearchEngine::addParam!");
    ENSURE_WARN(!this._readOnly,
                "called nsISearchEngine::addParam on a read-only engine!",
                Cr.NS_ERROR_FAILURE);
    ...

When user install Firefox 38 (or later), search.json will not include MozParam part and it should be updated. I don't know when it will be updated. If we always re-create search.json on Firefox update it's fine. But if we don't always update it, we need update it explicitly on the next Firefox update.

Please check when search.json file will be updated and add some more patch to do so if needed.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Kev, do you have plans for this? We are probably going to do a dot release for 40 this week but we won't wait for this patch.

Comment 9

[Mike Connor [:mconnor]]

Comment on attachment 8649393 [details] [diff] [review]
Patch to restore fr codes for use by Yahoo Search Plugins on Desktop

Approval Request Comment
[Feature/regressing bug #]: bug 1119560
[User impact if declined]: 
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low risk, restorej
[String/UUID change made/needed]:

tl;dr this removed partner codes for all non en-US locales for Yahoo, which is a Bad Thing for multiple reasons.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8649393 [details] [diff] [review]
Patch to restore fr codes for use by Yahoo Search Plugins on Desktop

OK, let's take it everywhere.

Comment 11

[Axel Hecht [:Pike]]

Note:

Taking on ESR: No idea what this means, tbh.

Taking on release: manual push to the release repo, probably manual edits for all upcoming fx40 releases.

Beta and downwards are easy, and can just follow standard landing and sign-off procedures.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/479f19c3a84e

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f5e52e74924e

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4a88f07fb036

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/dd527318d2f3

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/479f19c3a84e

Comment 17

[Axel Hecht [:Pike]]

btw, I realized by now that my thinking in comment 11 about what caused this bug was screwed, and we don't need any landings on the l10n side of things at all, sorry for the noise.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/768c402d0bca (ESR 38.2.x)
https://hg.mozilla.org/releases/mozilla-esr38/rev/1d2eb4187b5a (ESR 38.3.x)

Comment 19

[Nick Thomas [:nthomas] (UTC+12)]

Transplanted onto the Firefox relbranch for esr38 (GECKO3820esr_2015080613_RELBRANCH):
http://hg.mozilla.org/releases/mozilla-esr38/rev/74b253daf958

768c402d0bca is on the Thunderbird relbranch.

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes with "Some search partner codes were missing (1195683)" as wording.
Let me know if you want a different wording.

Comment 21

[Paul Silaghi, QA [:pauly]]

browser.search.param.yahoo-fr=moz35
browser.search.param.yahoo-fr-ja=mozff
are now displayed on FF 40.0.3, 42.0a2 (2015-08-27), 43.0a1 (2015-08-26), 38.2.1 ESR.

Comment 22

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 41b5.