Closed
Bug 794204
Opened 11 years ago
Closed 11 years ago
Add Yandex to default list of search plugins to mobile
Categories
(Mozilla Localizations :: ru / Russian, defect)
Tracking
(firefox16+ verified, firefox17 verified, firefox18 verified)
VERIFIED
FIXED
mozilla18
People
(Reporter: kev, Assigned: kbrosnan)
Details
Attachments
(1 file, 5 obsolete files)
3.73 KB,
patch
|
Pike
:
review+
|
Details | Diff | Splinter Review |
Yandex is the market leader for search in Russia, and has been suggested as an addition to Firefox Mobile since the original version was launched. This patch adds Yandex to the existing default list of search plugins for Firefox for Android.
Attachment #664616 -
Flags: review?(milos)
Attachment #664616 -
Flags: feedback?(krudnitski)
Comment 1•11 years ago
|
||
I think this is a good win for the Russian locale and our relationship with Yandex. I think it's important to provide local services where vetted and able in order to provide a better experience overall for those local users. I'd like to get this implemented ASAP into the list of search providers.
Comment 2•11 years ago
|
||
>diff -r 10acabb2e5f4 mobile/searchplugins/list.txt
>--- a/mobile/searchplugins/list.txt Sat Sep 22 19:13:52 2012 +0400
>+++ b/mobile/searchplugins/list.txt Mon Sep 24 20:00:39 2012 -0400
>@@ -1,3 +1,4 @@
> google
> twitter
>+yandex
> wikipedia-ru
I wonder, should it be yandex-ru? It's kind of customary to add locale code after name of search engine, if search engine has been added in several locales and differs between them. Currently desktop be, ru, tr and uk ship Yandex search plugins. be, tr and uk may want to add Yandex in list of mobile search plugins. Will it be all right to ship different Yandex search plugins with same name in multilocale Android build?
Reporter | ||
Comment 3•11 years ago
|
||
Alexander - I think we can do either or, especially given that there is a Yandex.com as well. I'll re-file the patch with yandex-ru. Do you or Konstantin have any feedback on adding Yandex to the list of providers on mobile in Ru?
Comment 4•11 years ago
|
||
(In reply to Kev [:kev] Needham from comment #3) > Alexander - I think we can do either or, especially given that there is a > Yandex.com as well. I'll re-file the patch with yandex-ru. Do you or > Konstantin have any feedback on adding Yandex to the list of providers on > mobile in Ru? I'm fine with adding Yandex to mobile for Ru. I assume, that you got this search plugin from Yandex? It looks almost identical to desktop one, there is nothing mobile specific. I wonder, why they have decided to use http://yandex.ru/ instead of http://m.yandex.ru/
Reporter | ||
Comment 5•11 years ago
|
||
currently m.yandex.ru redirects to yandex.ru results. we'll be asking them to redirect to the same page they use for the stock browser on android, which is also under yandex.ru, but optimized a little.
Reporter | ||
Comment 6•11 years ago
|
||
sorry, hit submit a little quickly. Yes, it's the same plugin as desktop, with a different CLID. They'll be doing UA sniffing and returning a results page based on that. The current search results aren't too bad as they are, and will be better with a tweak.
Comment 7•11 years ago
|
||
(In reply to Kev [:kev] Needham from comment #6) > sorry, hit submit a little quickly. Yes, it's the same plugin as desktop, > with a different CLID. They'll be doing UA sniffing and returning a results > page based on that. The current search results aren't too bad as they are, > and will be better with a tweak. Nice to hear. Please, test this plugin on phone and tablet, which have different UAs, to make sure that Yandex will provide optimized versions of pages for both kinds of UAs. I don't have Android tablet, so I can not help here.
Reporter | ||
Comment 8•11 years ago
|
||
Revised patch per comment #2. Milos/Alexander, please set reviewer as appropriate.
Attachment #664616 -
Attachment is obsolete: true
Attachment #664616 -
Flags: review?(milos)
Attachment #664616 -
Flags: feedback?(krudnitski)
Attachment #664747 -
Flags: review?(milos)
Updated•11 years ago
|
tracking-firefox16:
--- → +
Comment 9•11 years ago
|
||
Comment on attachment 664747 [details] [diff] [review] Add Yandex to Firefox Mobile RU locale Review of attachment 664747 [details] [diff] [review]: ----------------------------------------------------------------- Grabbing the r? from Milos, this looks good to me, r=me. Setting a r? on Alexander still. Alexander, with your review, please land on aurora and central? We'd want to take a build from aurora to put through QA and then transplant to beta. Thanks.
Attachment #664747 -
Flags: review?(unghost)
Attachment #664747 -
Flags: review?(milos)
Attachment #664747 -
Flags: review+
Updated•11 years ago
|
Attachment #664747 -
Flags: review?(unghost) → review+
Comment 10•11 years ago
|
||
Landed on aurora and central: http://hg.mozilla.org/releases/l10n/mozilla-aurora/ru/rev/ec0620b80327 http://hg.mozilla.org/l10n-central/ru/rev/5e382b309b73
Comment 11•11 years ago
|
||
We're going to re-spin nightlies in bug 795048, and Kevin is going to make sure that things are working as expected in English/Russian. Thanks all.
Assignee: nobody → kbrosnan
Comment 12•11 years ago
|
||
Once verified by QA, we'll want to land on Beta as well.
Comment 13•11 years ago
|
||
I've installed the Nightly multi-apk to at least to verify that the build didn't blow up (Axel's worry). Looks fine. Haven't verified that Yandex search is working however.
Reporter | ||
Comment 14•11 years ago
|
||
Alexander/Konstantin: can you give feedback on Yandex experience on Android?
Comment 15•11 years ago
|
||
(In reply to Kev [:kev] Needham from comment #14) > Alexander/Konstantin: can you give feedback on Yandex experience on Android? It works, but it shows desktop version of search. It's not optimized for mobile. Tested on HTC Desire with Android 2.3 with today's Aurora build.
Comment 16•11 years ago
|
||
I realized while reviewing other plugins, the <Image> for mobile search plugins is usually 32x32, Kev, would we have artwork for that? (The xml tags all say 16x16 on purpose, bug 795866.) I can tell a bit that the logo is scaled up, not sure if that matters.
Comment 17•11 years ago
|
||
Looks like all en-US mobile search plugins doesn't have <Description> field. Addon manager uses http://mxr.mozilla.org/mozilla-central/source/mobile/android/locales/en-US/chrome/aboutAddons.properties#11 for search plugins descriptions. Current view of description for Yandex plugin in addon manager is very long and looks insonsistent. I propose to remove it.
Attachment #666535 -
Flags: review?(l10n)
Reporter | ||
Comment 18•11 years ago
|
||
Concur with Alexander in c17, and will see if I can source a 32x32 yandex icon in short order. Patch will drop in next 60min or so. Thanks all.
Reporter | ||
Comment 19•11 years ago
|
||
Final version of adding Yandex to mobile search in ru. Removes <description> for consistency to address comment #17 and replaces search icon with 32x32 version to address comment #16. Axel/Aleksander, please review and lets land it :) Thanks again, everyone.
Attachment #664747 -
Attachment is obsolete: true
Attachment #666535 -
Attachment is obsolete: true
Attachment #666535 -
Flags: review?(l10n)
Attachment #666689 -
Flags: review?(l10n)
Reporter | ||
Comment 20•11 years ago
|
||
...and now with new and improved search plugin icon metadata (h/w) and mimetype.
Attachment #666689 -
Attachment is obsolete: true
Attachment #666689 -
Flags: review?(l10n)
Attachment #666694 -
Flags: review?(l10n)
Comment 21•11 years ago
|
||
Comment on attachment 666689 [details] [diff] [review] Add Yandex to Firefox Mobile RU locale Review of attachment 666689 [details] [diff] [review]: ----------------------------------------------------------------- kev, can you create a patch against the current state of the aurora repo? This patch seems to be against an old state, and makes it hard to figure out what's actually changing. Let alone to apply it.
Attachment #666689 -
Attachment is obsolete: false
Comment 22•11 years ago
|
||
Comment on attachment 666694 [details] [diff] [review] Add Yandex to Firefox Mobile RU locale Not sure what I r- just now. Doing that again. Also, <Image width="32" height="32"> needs to be <Image width="16" height="16"> despite it having a 32x32 url. The search service only picks up 16x16, but shows 32x32.
Attachment #666694 -
Attachment is patch: true
Attachment #666694 -
Flags: review?(l10n) → review-
Reporter | ||
Comment 23•11 years ago
|
||
Trying one more time :) Adding Yandex to Mobile, patched against Aurora to remove <description>, add 32x32 icon, but keep 16x16 h/w in meta data.
Attachment #666689 -
Attachment is obsolete: true
Attachment #666694 -
Attachment is obsolete: true
Attachment #666701 -
Flags: review?(l10n)
Comment 24•11 years ago
|
||
Comment on attachment 666701 [details] [diff] [review] Add Yandex to Firefox Mobile RU locale (Aurora Patch) Review of attachment 666701 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, this is looking good.
Attachment #666701 -
Flags: review?(l10n) → review+
Comment 25•11 years ago
|
||
Checked in central and aurora: http://hg.mozilla.org/l10n-central/ru/rev/cfa54fb83e02 http://hg.mozilla.org/releases/l10n/mozilla-aurora/ru/rev/c384a88aee42
Comment 26•11 years ago
|
||
Transplanted both changesets to beta, I'll add a note once the dashboard's ready. http://hg.mozilla.org/releases/l10n/mozilla-beta/ru/pushloghtml?changeset=1ad89374b742
Comment 27•11 years ago
|
||
Sign-off for 16 taken, Elvis has left the building.
Status: NEW → RESOLVED
Closed: 11 years ago
status-firefox16:
--- → fixed
status-firefox17:
--- → fixed
status-firefox18:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Assignee | ||
Comment 28•11 years ago
|
||
Verified on Nightly 18a1 2012-10-02 Aurora 17a2 2012-10-02 Beta 16b6 On new profiles and existing profiles when the system locale is set to Russian Yandex shows up as the second search provider. Typing a search query and pressing the address bar search button or the keyboard search or enter button performs a Google search. I assume that is expected?
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•