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)
Mozilla Localizations
Other
Tracking
(firefox46 fixed, firefox47 fixed)
RESOLVED
FIXED
mozilla47
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)
Comment 1•8 years ago
|
||
IIRC, the pref was added originally to only generate revenue for official builds, or something like that, and was desktop-first?
Reporter | ||
Comment 2•8 years ago
|
||
(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).
Comment 3•8 years ago
|
||
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)
Comment 4•8 years ago
|
||
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)
Reporter | ||
Comment 5•8 years ago
|
||
(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" />
Assignee | ||
Comment 6•8 years ago
|
||
OK, will starting looking in the next days, I expect to fix this on central/aurora before next week.
Assignee | ||
Comment 7•8 years ago
|
||
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
Assignee | ||
Comment 8•8 years ago
|
||
(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
Assignee | ||
Comment 9•8 years ago
|
||
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
Assignee | ||
Comment 10•8 years ago
|
||
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
Reporter | ||
Comment 11•8 years ago
|
||
(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! :-)
Reporter | ||
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•