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

VERIFIED FIXED in Firefox 40

Status

()

defect
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: bugzilla, Assigned: kev)

Tracking

({regression})

unspecified
Firefox 43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40+ verified, firefox41+ verified, firefox42+ verified, firefox43+ verified, firefox-esr31 unaffected, firefox-esr3840+ verified, relnote-firefox 40+)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
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
[Tracking Requested - why for this release]:
This is affiliate code issue for Yahoo! Japan.  We should fix this on release channel.
Keywords: regression
I mentioned this in our meeting just now. Sounds like we should enable the pref. Would we also do that for ESR?
Flags: needinfo?(sledru)
Assignee

Comment 3

4 years ago
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.
Assignee

Updated

4 years ago
Assignee: nobody → kev
Summary: Restore browser.search.param.yahoo-fr-ja pref for Yahoo! Japan searchplugin → Restore "fr" preferences for non-US Yahoo! searchplugins
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.
Flags: needinfo?(sledru)
Assignee

Comment 5

4 years ago
Attachment #8649393 - Flags: review?(dtownsend)
Attachment #8649393 - Flags: feedback?(mconnor)
Attachment #8649393 - Flags: review?(dtownsend) → review+
Assignee

Comment 6

4 years ago
Dynamis, this should address. look good?
Flags: needinfo?(bugzilla)
Reporter

Comment 7

4 years ago
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.
Flags: needinfo?(bugzilla)
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.
Flags: needinfo?(kev)
Attachment #8649393 - Flags: feedback?(mconnor) → feedback+
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.
Attachment #8649393 - Flags: approval-mozilla-esr38?
Attachment #8649393 - Flags: approval-mozilla-beta?
Attachment #8649393 - Flags: approval-mozilla-aurora?
Attachment #8649393 - Flags: approval-mozilla-release?
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.
Attachment #8649393 - Flags: approval-mozilla-release?
Attachment #8649393 - Flags: approval-mozilla-release+
Attachment #8649393 - Flags: approval-mozilla-esr38?
Attachment #8649393 - Flags: approval-mozilla-esr38+
Attachment #8649393 - Flags: approval-mozilla-beta?
Attachment #8649393 - Flags: approval-mozilla-beta+
Attachment #8649393 - Flags: approval-mozilla-aurora?
Attachment #8649393 - Flags: approval-mozilla-aurora+

Comment 11

4 years ago
ignore
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.
https://hg.mozilla.org/mozilla-central/rev/479f19c3a84e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43

Comment 17

4 years ago
ignore
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.
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.
Flags: qe-verify+
Added to the release notes with "Some search partner codes were missing (1195683)" as wording.
Let me know if you want a different wording.
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.
Flags: qe-verify+
Verified fixed FF 41b5.
Assignee

Updated

3 years ago
Flags: needinfo?(kev)
You need to log in before you can comment on or make changes to this bug.