Closed Bug 1247562 Opened 8 years ago Closed 8 years ago

Hardcode the yahoo-fr preference value into Yahoo search plugins

Categories

(Mozilla Localizations :: Other, defect)

defect
Not set
normal

Tracking

(firefox46 fixed, firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: florian, Assigned: flod)

References

Details

Telemetry data is showing some evidence that the browser.search.param.yahoo-fr preference is being abused. There's no obvious reason for this to be a preference, so we should just hardcode the value inside the Yahoo search plugins.

Given that this is already being abused, the sooner we fix the better, but given it's likely been in this state for years, there's no rush and a fix can ride the trains.

Concretely, this means that:

<MozParam name="fr" condition="pref" pref="yahoo-fr" />
should become:
<Param name="fr" value="moz35"/>

<MozParam name="fr" condition="pref" pref="yahoo-fr-ja" />
should become:
<Param name="fr" value="mozff"/>


Mike suggested that we may want to deprecate the <MozParam condition="pref"> feature altogether, in which case it may be a good idea to also replace the only other existing usage in the tree:
<MozParam name="nresults" condition="pref" pref="maxSuggestions" />
would become:
<Param name="nresults" value="4" />
This pref was added in bug 1001566, but it's not obvious that this needs to be a pref. Margaret, would you prefer to keep this a pref, or to have it hardcoded?
Flags: needinfo?(margaret.leibovic)
IIRC, the pref was added originally to only generate revenue for official builds, or something like that, and was desktop-first?
(In reply to Axel Hecht [:Pike] from comment #1)
> IIRC, the pref was added originally to only generate revenue for official
> builds, or something like that, and was desktop-first?

That's probably why we had it historically, but currently all the brandings (including the "unofficial" one) have the same values: https://dxr.mozilla.org/mozilla-central/search?q=yahoo-fr&case=true&=mozilla-central&redirect=true

Fennec has a different value, which is hardcoded (<Param name="fr" value="mozmob1" /> in the mobile/searchplugins/yahoo-*.xml files in the l10n repositories).
I defer to mconnor, since I don't know if there's a good reason why we're using a pref as opposed to making it hardcoded. In that bug, the description says:

"I'm going to make this a MozParam so we can share between plugins/tweak the value centrally."

But if we're not actually doing this, and we want to deprecate this MozParam feature, it should be fine to change it to be hardcoded.
Flags: needinfo?(margaret.leibovic) → needinfo?(mconnor)
I think we can just hardcode nresults now, if we're going to hardcode the other values.  It's working fine as-is.
Flags: needinfo?(mconnor)
(In reply to Mike Connor [:mconnor] from comment #4)
> I think we can just hardcode nresults now, if we're going to hardcode the
> other values.  It's working fine as-is.

Ok, so:

In browser/searchplugins/yahoo-*.xml:

<MozParam name="fr" condition="pref" pref="yahoo-fr" />
should become:
<Param name="fr" value="moz35"/>

<MozParam name="fr" condition="pref" pref="yahoo-fr-ja" />
should become:
<Param name="fr" value="mozff"/>


In mobile/searchplugins/yahoo-*.xml:

<MozParam name="nresults" condition="pref" pref="maxSuggestions" />
should become:
<Param name="nresults" value="4" />
OK, will starting looking in the next days, I expect to fix this on central/aurora before next week.
First pass (mobile searchplugins):
https://hg.mozilla.org/releases/l10n/mozilla-aurora/an/rev/1365c95b25e0
https://hg.mozilla.org/releases/l10n/mozilla-aurora/as/rev/d4b06143a090
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ast/rev/cbed242c35e0
https://hg.mozilla.org/releases/l10n/mozilla-aurora/bn-IN/rev/6b45fbc2d750
https://hg.mozilla.org/releases/l10n/mozilla-aurora/br/rev/9ac7dcc4e6e5
https://hg.mozilla.org/releases/l10n/mozilla-aurora/brx/rev/0cb0aa1d8362
https://hg.mozilla.org/releases/l10n/mozilla-aurora/cy/rev/8aac2dddd98b
https://hg.mozilla.org/releases/l10n/mozilla-aurora/de/rev/e7291736e620
https://hg.mozilla.org/releases/l10n/mozilla-aurora/dsb/rev/2383062b0c11
https://hg.mozilla.org/releases/l10n/mozilla-aurora/en-GB/rev/3194e1eb74bc
https://hg.mozilla.org/releases/l10n/mozilla-aurora/es-AR/rev/a15ad20eebd5
https://hg.mozilla.org/releases/l10n/mozilla-aurora/es-ES/rev/bb34f79a77b5
https://hg.mozilla.org/releases/l10n/mozilla-aurora/es-MX/rev/28863877b123
https://hg.mozilla.org/releases/l10n/mozilla-aurora/eu/rev/b628da01c276
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ff/rev/e9481470bb4d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/fi/rev/37c8f532fc96
https://hg.mozilla.org/releases/l10n/mozilla-aurora/fr/rev/97db21ce272a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/fy-NL/rev/b583cb6ad8de
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ga-IE/rev/6f6d710c73e8
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gd/rev/b7e85ba08244
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gl/rev/0940e9e32d2d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gn/rev/29a20306329a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gu-IN/rev/add981e5bcfe
https://hg.mozilla.org/releases/l10n/mozilla-aurora/hi-IN/rev/c050a0a21d67
https://hg.mozilla.org/releases/l10n/mozilla-aurora/hsb/rev/d42429aa573a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/id/rev/8b6015805d74
https://hg.mozilla.org/releases/l10n/mozilla-aurora/it/rev/f36c6c6ce136
https://hg.mozilla.org/releases/l10n/mozilla-aurora/kn/rev/e6cfb9afe01e
https://hg.mozilla.org/releases/l10n/mozilla-aurora/kok/rev/ff95e63bdd40
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ks/rev/a982f1168fa2
https://hg.mozilla.org/releases/l10n/mozilla-aurora/lij/rev/d46003cd897c
https://hg.mozilla.org/releases/l10n/mozilla-aurora/mai/rev/73fc321a8acd
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ml/rev/1b85d75836cd
https://hg.mozilla.org/releases/l10n/mozilla-aurora/mr/rev/99b720fac01d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/nb-NO/rev/bd330273c5ff
https://hg.mozilla.org/releases/l10n/mozilla-aurora/oc/rev/6808ea3b364d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/or/rev/bd3788ee024a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/pa-IN/rev/8fe3c8c83d25
https://hg.mozilla.org/releases/l10n/mozilla-aurora/pt-BR/rev/1fa05550131a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/rm/rev/e7dc4158b89d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/sat/rev/89b13ba2057f
https://hg.mozilla.org/releases/l10n/mozilla-aurora/son/rev/d8d171161b09
https://hg.mozilla.org/releases/l10n/mozilla-aurora/sv-SE/rev/ea1de38fe731
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ta/rev/edb2de35af3a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/te/rev/10c8ffb12b18
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ur/rev/862690a0c2e0
(In reply to Francesco Lodolo [:flod] from comment #7)
> First pass (mobile searchplugins):

Corrections: it's mostly mobile, but also some desktop searchplugins. Looks like the script I'm using is ignoring some searchplugins, need to figure out the reason.
Status: NEW → ASSIGNED
Found the issue: the replacement code was fine, but I was running "hg up -C" between two runs on the same locale, so I was only updating one searchplugin.

Will wait for MXR to update do check if there are any remnants.

https://hg.mozilla.org/releases/l10n/mozilla-aurora/an/rev/fc37ccf9f67b
https://hg.mozilla.org/releases/l10n/mozilla-aurora/as/rev/a460bf20b979
https://hg.mozilla.org/releases/l10n/mozilla-aurora/bn-IN/rev/a2ea6b87d80d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/br/rev/5101615f9de4
https://hg.mozilla.org/releases/l10n/mozilla-aurora/cy/rev/05f0c37c2919
https://hg.mozilla.org/releases/l10n/mozilla-aurora/de/rev/8e7958d14f60
https://hg.mozilla.org/releases/l10n/mozilla-aurora/dsb/rev/c35015b8b142
https://hg.mozilla.org/releases/l10n/mozilla-aurora/en-GB/rev/cdcd5e25e4ba
https://hg.mozilla.org/releases/l10n/mozilla-aurora/es-ES/rev/3562a3520be7
https://hg.mozilla.org/releases/l10n/mozilla-aurora/es-MX/rev/5cf90ab852a0
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ff/rev/16920499ef05
https://hg.mozilla.org/releases/l10n/mozilla-aurora/fi/rev/bbf47fb510f5
https://hg.mozilla.org/releases/l10n/mozilla-aurora/fr/rev/026b7a3bc33d
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gd/rev/e7efc5e5a51b
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gl/rev/044129546ee0
https://hg.mozilla.org/releases/l10n/mozilla-aurora/gu-IN/rev/974cdea10880
https://hg.mozilla.org/releases/l10n/mozilla-aurora/hi-IN/rev/ef4a16ce6f1b
https://hg.mozilla.org/releases/l10n/mozilla-aurora/hsb/rev/8cf85e4ca136
https://hg.mozilla.org/releases/l10n/mozilla-aurora/id/rev/11c55ca3e885
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ja/rev/4fa0c95d172b
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ja-JP-mac/rev/8e7ddfd68b3f
https://hg.mozilla.org/releases/l10n/mozilla-aurora/kn/rev/18654dc83e8a
https://hg.mozilla.org/releases/l10n/mozilla-aurora/mai/rev/c3aba51026cd
https://hg.mozilla.org/releases/l10n/mozilla-aurora/mr/rev/a44b5ede3294
https://hg.mozilla.org/releases/l10n/mozilla-aurora/or/rev/843a9f1a4941
https://hg.mozilla.org/releases/l10n/mozilla-aurora/pa-IN/rev/81950904f0a6
https://hg.mozilla.org/releases/l10n/mozilla-aurora/pt-BR/rev/966964112281
https://hg.mozilla.org/releases/l10n/mozilla-aurora/rm/rev/0a52db6a6117
https://hg.mozilla.org/releases/l10n/mozilla-aurora/son/rev/3ae9a4ff0453
https://hg.mozilla.org/releases/l10n/mozilla-aurora/ta/rev/e152d6ab0808
https://hg.mozilla.org/releases/l10n/mozilla-aurora/te/rev/286ff9885ea2
I see some Suite and Thunderbird searchplugins still using that setting, but nothing for Firefox or Fennec, so I consider this bug fixed (not sure if you want to file a follow-up, but that wouldn't be for me to fix).

http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=condition%3D%22pref%22&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=l10n-mozilla-aurora

l10n-central still reports es-CL, because I missed a manual transplant (one of the few locales needing that)
http://hg.mozilla.org/l10n-central/es-CL/rev/b1b37dd12df4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(In reply to Francesco Lodolo [:flod] from comment #10)
> I see some Suite and Thunderbird searchplugins still using that setting

The results for mail/ on mxr are actually commented out lines in the ja and ja-JP-mac locales.

I don't think the usage in suite/ actually works, because the browser.search.param.yahoo-fr prefs only exist in the browser/ brandings: http://mxr.mozilla.org/comm-central/search?string=yahoo-fr

My guess is these lines in the suite/ search plugins are just dead code.

> nothing for Firefox or Fennec, so I consider this bug fixed

Thank you! :-)
Target Milestone: --- → mozilla47
Blocks: 1251278
You need to log in before you can comment on or make changes to this bug.