Closed
Bug 1200170
Opened 9 years ago
Closed 9 years ago
Yahoo! Japan searchplugin for Fennec should include affiliate code with Param, not MozParam
Categories
(Mozilla Localizations :: ja / Japanese, defect)
Mozilla Localizations
ja / Japanese
Tracking
(firefox40- wontfix, firefox41+ fixed, firefox42+ fixed, firefox43+ fixed, firefox-esr31 wontfix, firefox-esr38 wontfix, fennec41+)
People
(Reporter: bugzilla, Assigned: bugzilla)
Details
Attachments
(1 file)
2.65 KB,
patch
|
flod
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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)
Assignee | ||
Comment 2•9 years ago
|
||
[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: --- → ?
status-firefox40:
--- → affected
status-firefox41:
--- → affected
status-firefox42:
--- → affected
status-firefox43:
--- → affected
status-firefox-esr31:
--- → unaffected
status-firefox-esr38:
--- → unaffected
tracking-firefox40:
--- → ?
tracking-firefox41:
--- → ?
tracking-firefox42:
--- → ?
tracking-firefox43:
--- → ?
Updated•9 years ago
|
Assignee: nobody → bugzilla
Component: Search Activity → ja / Japanese
Product: Firefox for Android → Mozilla Localizations
QA Contact: l10n-qa
Comment 3•9 years ago
|
||
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 4•9 years ago
|
||
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+
Updated•9 years ago
|
Keywords: productwanted
Assignee | ||
Comment 5•9 years ago
|
||
Thanks! Our l10n team will land this patch and sign-off for it soon.
Comment 6•9 years ago
|
||
(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.
Comment 7•9 years ago
|
||
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.
Updated•9 years ago
|
Keywords: productwanted
Comment 8•9 years ago
|
||
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
Comment 9•9 years ago
|
||
(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).
Assignee | ||
Comment 10•9 years ago
|
||
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?
Flags: needinfo?(francesco.lodolo)
Comment 12•9 years ago
|
||
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: 9 years ago
Flags: needinfo?(francesco.lodolo)
Resolution: --- → FIXED
Comment 13•9 years ago
|
||
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/
Comment 15•9 years ago
|
||
As far as I can tell the two ESR are wontfix at this point.
Updated•9 years ago
|
Comment 16•9 years ago
|
||
(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
Flags: needinfo?(francesco.lodolo)
Comment 17•9 years ago
|
||
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
Flags: needinfo?(francesco.lodolo)
Comment 18•9 years ago
|
||
(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.
Description
•