Closed Bug 1200170 Opened 4 years ago Closed 4 years ago
Yahoo! Japan searchplugin for Fennec should include affiliate code with Param, not Moz
Yahoo! Japan search plug-in for Fennec should include affiliate code fr=mozff: http://search.yahoo.co.jp/search?p=test&ei=UTF-8&fr=mozff To do so, let's update search plug-in for Fennec: http://hg.mozilla.org/l10n-central/ja/file/tip/mobile/searchplugins/yahoo-jp.xml - <MozParam name="fr" condition="pref" pref="yahoo-fr-ja" /> + <Param name="fr" value="mozff" /> Background: For desktop Firefox, we use <MozParam> in the plug-in and we have yahoo-fr-ja pref defined in the prefs (bug 1195683): http://hg.mozilla.org/l10n-central/ja/file/tip/browser/searchplugins/yahoo-jp.xml <MozParam name="fr" condition="pref" pref="yahoo-fr-ja" /> https://dxr.mozilla.org/mozilla-central/source/browser/branding/nightly/pref/firefox-branding.js#32 pref("browser.search.param.yahoo-fr", "moz35"); pref("browser.search.param.yahoo-fr-ja", "mozff"); For mobile Fennec, we use same search plug-in as desktop http://hg.mozilla.org/l10n-central/ja/file/tip/mobile/searchplugins/yahoo-jp.xml but unfortunately <MozParam> support is WONTFIX (bug 1062397). As a result, current search URL is: http://search.yahoo.co.jp/search?p=test&ei=UTF-8 To include affiliate code, we need do one of: A. just change from <MozParam> to <Param> B. support <MozParam> and add pref for it I recommend to do A since it's much easier, less accident like bug 1195683 and we need not use pref for affiliate code now. Note: In the past we use <MozParam> to send affiliate code pending on our version (fr=moz35 for Firefox 3.5). But currently we always send same code fr=mozff. We need not read value from prefs. So I suggest just to change from <MozParam> to <Param> for Fennec search plug-in.
This is the patch to update Yahoo! Japan search plug-in for ja Fennec. When you give approve for this, I'll request ja locale leader to include this patch and and sign-off for the next releases.
Attachment #8654814 - Flags: review?(l10n)
[Tracking Requested - why for this release]: This is the affiliate code issue. This must be fixed before the next release. Please give approval for this.
tracking-fennec: --- → ?
Assignee: nobody → bugzilla
Component: Search Activity → ja / Japanese
Product: Firefox for Android → Mozilla Localizations
QA Contact: l10n-qa
Since the fix is going to happen in the l10n repository, I don't think it makes sense to have this in the Firefox for Android module. CCing Mike Connor so he's aware of the change and can shout if he doesn't agree with the change.
Comment on attachment 8654814 [details] [diff] [review] change from <MozParam> to <Param> (for ja, same for ja-JP-mac for consistency) Review of attachment 8654814 [details] [diff] [review]: ----------------------------------------------------------------- Stealing flag from Pike. The change is reasonable and works as expected. @Mike: As said please let me know if you think this should be fixed otherwise.
Attachment #8654814 - Flags: review?(l10n) → review+
Thanks! Our l10n team will land this patch and sign-off for it soon.
(In reply to dynamis (Tomoya ASAI) from comment #5) > Thanks! Our l10n team will land this patch and sign-off for it soon. Consider that Fx41 (beta cycle) is currently already past sign-off (deadline is 2 weeks before merge day). Given how long this has affected us, I guess it's OK to target Firefox 42.
For some clarity, the MozParam WONTFIX only covers the Search Activity, not the core plugin. The reason this doesn't work in core search is because Fennec doesn't have the right pref. We should take and uplift this into 41, Fennec drivers are aware and we should get this into 41 if at all possible.
I pushed the change to all repositories for both ja an ja-JP-mac (for consistency). https://hg.mozilla.org/releases/l10n/mozilla-release/ja/rev/11a3bc55d119 https://hg.mozilla.org/releases/l10n/mozilla-beta/ja/rev/11a3bc55d119 https://hg.mozilla.org/releases/l10n/mozilla-aurora/ja/rev/11a3bc55d119 http://hg.mozilla.org/l10n-central/ja/rev/11a3bc55d119 https://hg.mozilla.org/releases/l10n/mozilla-release/ja-JP-mac/rev/749c9d48c870 https://hg.mozilla.org/releases/l10n/mozilla-beta/ja/rev/749c9d48c870 https://hg.mozilla.org/releases/l10n/mozilla-aurora/ja/rev/749c9d48c870 http://hg.mozilla.org/l10n-central/ja/rev/749c9d48c870
(In reply to Francesco Lodolo [:flod] from comment #8) > I pushed the change to all repositories for both ja an ja-JP-mac (for > consistency). Side note: the xml file in ja-JP-mac was out of sync (icon).
Thank you very much Mike, flod and all! (In reply to Francesco Lodolo [:flod] from comment #8) > https://hg.mozilla.org/releases/l10n/mozilla-release/ja-JP-mac/rev/ > 749c9d48c870 > https://hg.mozilla.org/releases/l10n/mozilla-beta/ja/rev/749c9d48c870 > https://hg.mozilla.org/releases/l10n/mozilla-aurora/ja/rev/749c9d48c870 > http://hg.mozilla.org/l10n-central/ja/rev/749c9d48c870 Actually: https://hg.mozilla.org/releases/l10n/mozilla-release/ja-JP-mac/rev/749c9d48c870 https://hg.mozilla.org/releases/l10n/mozilla-beta/ja-JP-mac/rev/749c9d48c870 https://hg.mozilla.org/releases/l10n/mozilla-aurora/ja-JP-mac/rev/749c9d48c870 http://hg.mozilla.org/l10n-central/ja-JP-mac/rev/749c9d48c870
Tracked. Flod, could you please mark this bug as fixed based on the fact that the fixes landed in m-r branch?
It's definitely fixed, but it would be nice to have someone to confirm that the RC actually contains this fix.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
This build contains the right searchplugin, I guess that's enough to state that we're good. http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/41.0b10-candidates/build1/android-api-11/ja/
Can we change the affected flags to match reality?
tracking-fennec: ? → 41+
As far as I can tell the two ESR are wontfix at this point.
(In reply to Francesco Lodolo [:flod] from comment #15) > As far as I can tell the two ESR are wontfix at this point. Francesco, could you uplift this to beta? Although you mark that status-firefox42 is fixed, this isn't landed on beta. http://hg.mozilla.org/releases/l10n/mozilla-beta/ja/log/84853040d6b2/mobile/searchplugins/yahoo-jp.xml
This *has* landed in beta http://hg.mozilla.org/releases/l10n/mozilla-beta/ja/log/default/mobile/searchplugins/yahoo-jp.xml You're looking at the revision tagged for Firefox Desktop, which doesn't have the fix (and doesn't need to). This one has been signed-off and used for the last Fennec Beta http://hg.mozilla.org/releases/l10n/mozilla-beta/ja/log/b8c4ac479d27/mobile/searchplugins/yahoo-jp.xml
(In reply to Francesco Lodolo [:flod] from comment #17) > This *has* landed in beta > http://hg.mozilla.org/releases/l10n/mozilla-beta/ja/log/default/mobile/ > searchplugins/yahoo-jp.xml > > You're looking at the revision tagged for Firefox Desktop, which doesn't > have the fix (and doesn't need to). This one has been signed-off and used > for the last Fennec Beta > http://hg.mozilla.org/releases/l10n/mozilla-beta/ja/log/b8c4ac479d27/mobile/ > searchplugins/yahoo-jp.xml ah, OK. I misunderstand. Thank you for verify.
You need to log in before you can comment on or make changes to this bug.