Closed Bug 1403108 Opened 5 years ago Closed 5 years ago

[cs] Remove Twitter search module from mobile

Categories

(Mozilla Localizations :: cs / Czech, enhancement)

enhancement
Not set
normal

Tracking

(firefox57 wontfix, firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: mstanke, Assigned: mstanke)

References

()

Details

Attachments

(2 files, 1 obsolete file)

Twitter search is the only search engine special for Fennec. All other engines are unified between all products.
How important is the Twitter search engine? Are we obligated or recommended to keep it?
Flags: needinfo?(francesco.lodolo)
Delphine is the PM in charge of mobile products, redirecting NI.
Flags: needinfo?(francesco.lodolo) → needinfo?(lebedel.delphine)
Hey Michal. No, Twitter isn't an obligation. I can remove it when I get back next week. I'll leave the ni on me to take care of this.
I probably know, where to remove it from Fennec, but we should probably remove it from everywhere (all Firefoxes for Android, iOS and Focuses).
Summary: [cs] Consider removing Twitter search module from Fennec → [cs] Consider removing Twitter search module from mobile
l10n "controls" the search engines only on Firefox for Android. If you want to take a shot, go for it :) Documentation is here: https://github.com/mozilla-l10n/documentation/blob/master/products/searchplugins/setup_searchplugins.md
Michal: you wanted to work on the patch from what I understand in comment 4. Let me know if I misunderstood or if that's the case (no rush, just want to make sure I got it). Thanks
Flags: needinfo?(lebedel.delphine) → needinfo?(mstanke)
Sorry for my inactivity, I have lost track of this ticket. Assigning myself to create a patch before the weekend.
Assignee: mvasicek → mstanke
Status: NEW → ASSIGNED
Flags: needinfo?(mstanke)
Comment on attachment 8918179 [details]
Bug 1403108 - [cs] Unify mobile search engines list with browser,

https://reviewboard.mozilla.org/r/189048/#review194610

Thanks Michal! There are a few things to go over I think. First of all I didn't realize that you didn't have Yahoo and Bing listed in your list.json, and we are now required to have them. They should appear first as "google", "yahoo", "bing" and the rest of the searchplugins should rather be in alphabetical order. It seems like the wikipedia.xml is a bit outdated too and could use a refresh (not sure how important it is to update it, but probably it's best). Since you're going to add Yahoo and Bing, you're also going to have to edit the mobile_region.properties file as well. All the details can be found here: https://github.com/mozilla-l10n/documentation/blob/master/products/searchplugins/setup_searchplugins.md  Thank you!
Attachment #8918179 - Flags: review?(lebedel.delphine) → review-
Hi :delphine.

Is it really necessary to add those two? I remember we have removed them on purpose in the past, because they did not work well for Czech queries. I see it got better, but Yahoo still yields results mostly in English, and the interface is in English too.
Flags: needinfo?(lebedel.delphine)
On extra note: adding Yahoo and Bing we will end up having 4 generic search engines - Google, Yahoo, Bing and Seznam...
Hey Michal - thanks for raising those valid points, I wasn't aware of that. I'll check in and see if this is still a hard requirement or not!
Flags: needinfo?(lebedel.delphine)
Hi Mike: when I started working on l10n mobile, I was told the requirements for Firefox Android searchengines were "google, yahoo, bing". 
It's been a while now, and I'm wondering if these "hard requirements" had shifted at all since then. Especially when I read comment 10, it doesn't seem to make much sense to add them there. I believe there were other cases in the recent past when this seemed to not make much sense anymore too. Let me know, thanks!
Flags: needinfo?(mozilla)
Moving needinfo over to Mike Connor who will know more about our actual requirements of engines in particular countries.
Flags: needinfo?(mozilla) → needinfo?(mconnor)
Hey Michal - so flod and I met with Joanne yesterday (from Business Development, I've CCed her here in case you had any questions) and it seems like there are global deals Mozilla has and that we can not break in this case. Joanne advised that we should be adding Bing to the existing list here, at the very least (it's OK to drop Yahoo).

This should also be reflected on desktop as a matter of fact. Please let Joanne and/or I know here if you have any questions about this, and we can try to help clarify. 

Otherwise, jumping back on my review comments from comment 9: wikipedia.xml is fine, no need to update it ;) Please ignore that part.

thanks Michal!
Flags: needinfo?(mconnor)
I want to be super explicit here - we are only removing Yahoo for this particular locale because the results are in English and not localized.  We'll be reviewing any changes in the future on a case by case basis.  thanks everyone!
Update for region.properties.
Attachment #8923172 - Flags: review?(lebedel.delphine)
Comment on attachment 8923172 [details] [diff] [review]
Bug 1403108 - Add Bing search engine for mobile

Review of attachment 8923172 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/chrome/region.properties
@@ +13,5 @@
>  # cause Firefox to re-read these prefs and inject any new handlers into the
>  # profile database.  Note that "new" is defined as "has a different URL"; this
>  # means that it's not possible to update the name of existing handler, so
>  # don't make any spelling errors here.
> +gecko.handlerService.defaultHandlersVersion=4

No need to update the handler version if you only change search.
Comment on attachment 8918179 [details]
Bug 1403108 - [cs] Unify mobile search engines list with browser,

https://reviewboard.mozilla.org/r/189048/#review200218

lgtm!
Attachment #8918179 - Flags: review?(lebedel.delphine) → review+
Attachment #8923185 - Flags: review?(lebedel.delphine) → review+
Thank you Delphine. One extra question, in which order should be these patches landed? Does it matter we have x-channel now and the l10n-central change would go straight to beta two weeks before the merge day?
Flags: needinfo?(lebedel.delphine)
This is an interesting case for cross-channel. Luckily, it's not a sensitive change.

l10n-central will have Bing set as 2nd searchplugin, no problems there.
If we ship this change in Beta, it will try to set Bing as 2nd searchplugin, but the file won't be enabled for cs.

I'd suggest to land the one in l10n-central/cs starting from tomorrow, so it doesn't affect 57. We can land the mozilla-central anytime (I'll trigger the landing).
Flags: needinfo?(lebedel.delphine)
Comment on attachment 8918179 [details]
Bug 1403108 - [cs] Unify mobile search engines list with browser,

https://reviewboard.mozilla.org/r/189048/#review200356

(need a l3 review to land)
Attachment #8918179 - Flags: review+
Pushed by francesco.lodolo@mozillaitalia.org:
https://hg.mozilla.org/integration/autoland/rev/45ce5a8174d0
[cs] Unify mobile search engines list with browser, r=delphine,flod
NI myself to not forgot and push it to l10n-central/cs later tomorrow.

Thank you for pushing the other patch to autoland.
Flags: needinfo?(mstanke)
https://hg.mozilla.org/mozilla-central/rev/45ce5a8174d0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
https://hg.mozilla.org/l10n-central/cs/rev/b75bcf5ed14e
Flags: needinfo?(mstanke)
Summary: [cs] Consider removing Twitter search module from mobile → [cs] Remove Twitter search module from mobile
You need to log in before you can comment on or make changes to this bug.