850984 - Request for set Baidu search engine as default for zh-CN Version of Firefox for Android

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Peipei Cheng (needinfo if you need my action)]

Google search service is very unstable in China mainland due to the "Great FireWall". 
It will be very important to set "Baidu" as the default search engine for zh-CN version for Firefox in Android.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Search engines can be set as part of the localization. We need to talk to the L10N team for zh-CN.

Comment 2

[Hong Tang]

Let me loop in our community localization guy Shaohua here.

Comment 3

[Shaohua Wen]

I'm fine with this change, and I would like to know if there is simple way to change the default search engine in Firefox for Android? in case some users might want to stick to the google thing even in case it's blocked frequently by GFW, we should provide then with this possibility.

Comment 4

[Axel Hecht [:Pike]]

More questions:

Should we keep google?

Next steps:

Add baidu to Firefox for Android. This likely starts with a in-country review and test of the desktop plugin. Same parameters to send, do urls change? We probably want a 32x32 version of the logo. Peipei, can you help with that?

Evaluate the impact of the settings in region.properties. I honestly don't know which of them are used, and which impact they have.

Comment 5

[Karen Rudnitski [:kar]]

Yes, keep google. We are only changing the default search engine and are not removing anything with this change.

Comment 6

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: 32X32 search logo of baidu.com

32X32 search logo of baidu.com

Comment 7

[Yuan Xulei [:yxl] (Leave from May 18)]

Hi @Axel Hecht, the search and suggestion urls are slightly different from those of the desktop. People use m.baidu.com instead of www.baidu.com on mobile devices:

# suggestion url 
 http://m.baidu.com/su?from=1000969a&wd={searchTerms}&ie=utf-8&action=opensearchh 

# search url
 http://m.baidu.com/s?from=1000969a&word={searchTerms}

"from=1000969a" is the partner ID of Mozilla Online, which Baidu.com uses to monitor the search traffic from us.

I could provide an XML file of the search plug-in for baidu.com if needed. ;-)

Comment 8

[Axel Hecht [:Pike]]

Yuan, does baidu do a got detection of mobile vs. desktop sites?

For context, we just un-hardcoded the wikipedia plugin from mobile-only to auto-detection, so that people on tablets for example can use the desktop version of the site.

Comment 9

[Yuan Xulei [:yxl] (Leave from May 18)]

Axel, no, the search url of baidu doesn't detect  the mobile vs desktop sites. 
Thus if the people on tables search using m.baidu.com, they cannot use the desktop version site.

Comment 10

[Axel Hecht [:Pike]]

Great, thanks for the feedback.

Milos, can you create a patch?

Comment 11

[Milos Dinic [:Milos]]

Before I start with a patch, I want to ask whether we want Baidu, as in www.baidu.com, or baiduzhidao, as in zhidao.baidu.com. For desktop, we're using both, I'm wondering if we want the same for mobile, as well.

Baidu and BaiduZhidao differ in the name and parameters that we're sending to their server, and everything else is pretty much the same, for what it's worth.

Comment 12

[Hong Tang]

We only need Baidu, as in  www.baidu.com for Firefox Android, since BaiduZhidao doesn't have good experience on mobile for now.
Thanks !

Comment 13

[bhavana bajaj [:bajaj]]

(In reply to Milos Dinic [:Milos] from comment #11)
> Before I start with a patch, I want to ask whether we want Baidu, as in
> www.baidu.com, or baiduzhidao, as in zhidao.baidu.com. For desktop, we're
> using both, I'm wondering if we want the same for mobile, as well.
> 
> Baidu and BaiduZhidao differ in the name and parameters that we're sending
> to their server, and everything else is pretty much the same, for what it's
> worth.

Looks like the tracking flags were unintentionally reset, i am setting them back.

Comment 14

[Milos Dinic [:Milos]]

Attachment: Adding Baidu

I've copied over baidu.xml from /browser/searchplugins/, changed bits as per comment 7, made it default search engine, made Google second in search dropdown and changed url for location bar searhes.

This patch is made against aurora. Please do not commit this patch. We'll wait for Axel to review it, and upon getting r+, I'll commit this to both aurora and beta repos.

Thanks.

Comment 15

[Axel Hecht [:Pike]]

Comment on attachment 734202 [details] [diff] [review]
Adding Baidu

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

The entries in region.properties have 4 glyphs instead of two, thus r-.

Also, please fix the whitespace in the xml.

::: mobile/chrome/region.properties
@@ +7,3 @@
>  
>  # Search engine order (order displayed in the search bar dropdown)s
> +browser.search.order.1=百度知道

These two should just be the ShortName, ie, 百度.

::: mobile/searchplugins/baidu.xml
@@ +14,5 @@
> +	</Url>
> +	<Url type="text/html" method="GET" template="http://m.baidu.com/s">
> +		<Param name="from" value="1000969a"/>
> +	    <Param name="word" value="{searchTerms}"/>
> +		</Url>

As we're talking, fix the indention here? I'm seeing tabs, which are generally frowned upon, please use ' ' whitespace only.

Comment 16

[Wennan Zhu]

Comment on attachment 734202 [details] [diff] [review]
Adding Baidu

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

::: mobile/searchplugins/baidu.xml
@@ +3,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
> +	<ShortName>百度</ShortName>
> +	<Description>百度网页搜索</Description>
> +	<InputEncoding>GB2312</InputEncoding>

InputEncoding should be UTF-8 here.

Comment 17

[Milos Dinic [:Milos]]

Attachment: Adding Baidu, take 2

Fixing region.properties to use shortname from baidu, changing encoding, and changing tabs for spaces.

Comment 18

[Wennan Zhu]

Hi Axel,

We are awared that Baidu has a strong requirement not to mix up  channel identifications for tablet (which is same as for PC) and mobile device.

For tablet:

# suggestion url 
 http://m.baidu.com/su?from=1000969a&wd={searchTerms}&ie=utf-8&action=opensearchh 

# search url
 http://www.baidu.com/baidu?tn=monline_4_dg&ie=utf-8&wd={searchTerms}

For mobile device: 
    
# suggestion url 
 http://m.baidu.com/su?from=1000969a&wd={searchTerms}&ie=utf-8&action=opensearchh 

# search url
 http://m.baidu.com/s?from=1000969a&word={searchTerms}


According to Baidu people, the definition is: 

    The device screen greater than 7 inches is defined as a tablet,
    while less or equal than 7 inches as a mobile device.


It seems that we need to distinguish those two types of device and use different urls as given.

Not sure how difficult it is for us to do that. Please let me know for any more information.

Thanks!

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wennan Zhu from comment #18)
> Hi Axel,
> 
> We are awared that Baidu has a strong requirement not to mix up  channel
> identifications for tablet (which is same as for PC) and mobile device.
> 
> For tablet:
> 
> # suggestion url 
>  http://m.baidu.com/su?from=1000969a&wd={searchTerms}&ie=utf-
> 8&action=opensearchh 
> 
> # search url
>  http://www.baidu.com/baidu?tn=monline_4_dg&ie=utf-8&wd={searchTerms}
> 
> For mobile device: 
>     
> # suggestion url 
>  http://m.baidu.com/su?from=1000969a&wd={searchTerms}&ie=utf-
> 8&action=opensearchh 
> 
> # search url
>  http://m.baidu.com/s?from=1000969a&word={searchTerms}
> 
> 
> According to Baidu people, the definition is: 
> 
>     The device screen greater than 7 inches is defined as a tablet,
>     while less or equal than 7 inches as a mobile device.
> 
> 
> It seems that we need to distinguish those two types of device and use
> different urls as given.
> 
> Not sure how difficult it is for us to do that. Please let me know for any
> more information.

We don't have that capability. We can only use a single URL. Some search providers only handle one URL and we pick "mobile". Other search providers uses some detection/sniffing to redirect to the mobile or tablet URL on it's own.

Comment 20

[Pin Zhang [:pzhang] (inactive)]

(In reply to Mark Finkle (:mfinkle) from comment #19)
> > 
> > Not sure how difficult it is for us to do that. Please let me know for any
> > more information.
> 
> We don't have that capability. We can only use a single URL. Some search
> providers only handle one URL and we pick "mobile". Other search providers
> uses some detection/sniffing to redirect to the mobile or tablet URL on it's
> own.

I found that we build the different UA strings for Mobile and Tablet[1], so is it possible to dynamically choose the search URL based on the same logic? 

[1] https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#642

Comment 21

[Axel Hecht [:Pike]]

We would need to extend the toolkit search service, and our search markup, https://developer.mozilla.org/en-US/docs/Creating_MozSearch_plugins.

I'd take it that that would be hard in the Firefox 21 timeframe. It might even turn into a Firefox 23 or later feature, assuming it would ride the trains.

Comment 22

[Hong Tang]

That's really great, if we can get it fixed.
I just talked to our DB people, they are okay to wait for a couple of release later.
Somehow baidu strongly requests us to use different URLs on different devices.

Comment 23

[Karen Rudnitski [:kar]]

Confirmed by BD in an email exchange with Cathy Zhou Apr 12 to wait for a solution to differentiate between tablet and smartphone as per Baidu's set requirements. It is understood that it may therefore not be implemented until late this year, but this is the preferred approach: to satisfy Baidu requirements.

Comment 24

[Axel Hecht [:Pike]]

Unsetting a host of flags then, I'll also r- based on that input.

Comment 25

[Axel Hecht [:Pike]]

I filed bug 861164 for the search service support for this, there are also further questions on what to expose precisely.

I'm unassigning this from Milos, too, as this bug's not actionable right now. Let's find an assignee once it is.

Comment 26

[Erin Lancaster [:elan]]

nom'd to change tracking flag.

Comment 27

[Erin Lancaster [:elan]]

Now that bug 861164 has been resolved, what are the next steps?

Comment 28

[Axel Hecht [:Pike]]

Holywen, do you want to start the implementation of this on central or aurora?

I see that you're occasionally merging aurora back to central, but not the other way around, so it might be best fitting your work flow if we wait until 24 is on aurora, then change the plugin, and then merge that back to central again?

Comment 29

[Axel Hecht [:Pike]]

Now that the search backend is on aurora, we should get this going again.

Milos, can you take this again?

Comment 30

[Shaohua Wen]

Hi Acel(In reply to Axel Hecht [:Pike] from comment #28)
> Holywen, do you want to start the implementation of this on central or
> aurora?
> 

Lets wait until 24 is on aurora, then change the plugin, and then merge that back to central again.
Thanks for your great proposal :)

Comment 31

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Shaohua Wen from comment #30)
> Hi Acel(In reply to Axel Hecht [:Pike] from comment #28)
> > Holywen, do you want to start the implementation of this on central or
> > aurora?
> > 
> 
> Lets wait until 24 is on aurora, then change the plugin, and then merge that
> back to central again.
> Thanks for your great proposal :)

Fx24 is currently on Aurora

Comment 32

[Shaohua Wen]

So will Milos land this patch or should I do it?

Comment 33

[Axel Hecht [:Pike]]

Milos will make a patch, talked to him in real-life. The one we have doesn't use the new search features yet, so we can't use that.

Comment 34

[Milos Dinic [:Milos]]

Attachment: Revised patch

Added <URL> with conditional(type) attribute that will make client use that one over the one with type="text/html" if a user is using a tablet. In other cases, it'll just use <URL> with with type="text/html".

Comment 35

[Axel Hecht [:Pike]]

Comment on attachment 772780 [details] [diff] [review]
Revised patch

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

The region.properties chunk will conflict with http://hg.mozilla.org/releases/l10n/mozilla-aurora/zh-CN/diff/9821c10b555a/mobile/chrome/region.properties, which catches up on the removal of keyword.URL.

Apart from that, r=me.

::: mobile/chrome/region.properties
@@ +18,5 @@
>  browser.contentHandlers.types.2.title=Google
>  browser.contentHandlers.types.2.uri=http://fusion.google.com/add?feedurl=%s
>  
>  # Keyword URL (for location bar searches)
> +keyword.URL=http://m.baidu.com/s?from=1000969a&word=

This should merge-conflict, AFAICT.

Comment 36

[Milos Dinic [:Milos]]

Thanks. Landed on aurora in http://hg.mozilla.org/releases/l10n/mozilla-aurora/zh-CN/rev/35c864a75887 .

Comment 37

[Peipei Cheng (needinfo if you need my action)]

Thanks a lot for all your guys' help on this! I will try this out as soon as next aurora build come out. Thanks again for making this happen!

Comment 38

[Peipei Cheng (needinfo if you need my action)]

Attachment: Lack of Baidu icon

I did some tests. The feature works for me. But the search plugin lack of Baidu icon. Could you please double check and use the icon in attachment 726065 [details]: 32X32 search logo of baidu.com if it is missing?

Comment 39

[Francesco Lodolo [:flod]]

Interesting. If I install the searchplugin on Firefox desktop I can see the icon, and the base64 image here seems correct.

Comment 40

[Axel Hecht [:Pike]]

(In reply to pcheng from comment #38)
> Created attachment 773750 [details]
> Lack of Baidu icon
> 
> I did some tests. The feature works for me. But the search plugin lack of
> Baidu icon. Could you please double check and use the icon in attachment
> 726065 [details]: 32X32 search logo of baidu.com if it is missing?

as flod, I'd expect this to work. Is there anything in the error console or adb logcat?

Comment 41

[Francesco Lodolo [:flod]]

Doing some tests, I only wish managing searchplugins was easier on Android. 

I can confirm the problem om my Nexus 7, and I'm pretty sure it doesn't like the 32x32 icon (trying to figure out if it's because is resized or just too big).

Comment 42

[Francesco Lodolo [:flod]]

Attachment: Test results

Test page
http://flod.org/baidu/

I tried on Firefox Aurora so that I could wipe the data out when finished (no idea how to remove searchplugins without access to the profile folder).

Result: only the 16x16 icon shows up.

Comment 43

[Peipei Cheng (needinfo if you need my action)]

I didn't see any specific error in adb logcat. And I can reproduce on Sumsung Note 8, Sunsung Galaxy S3 i9300, Sumsung GPT7510.

If it is the icon's problem, please let us know so we can provide the right ones.

Comment 44

[Francesco Lodolo [:flod]]

Not sure what the problem is, but I've updated the test page with a fourth searchplugin.

Converted the logo attached to this bug to base64, like this the searchplugin's icon is displayed correctly.

Comment 45

[Axel Hecht [:Pike]]

Found it, same as in Hungarian. desktop urldecodes the data uri, mobile doesn't. I need to file a bug on that. Any data uri with %2F in it etc breaks.

Comment 46

[Axel Hecht [:Pike]]

That said, flod, can you create a patch to use the other data uri for the image?

Comment 47

[Francesco Lodolo [:flod]]

Attachment: Change Baidu's icon

Patch using the attached image converted to base64

Comment 48

[Peipei Cheng (needinfo if you need my action)]

As I know the favicon for Google/wikipedia is also 32*32. And they can be shown correctly. Does anyone know why this is different?

Comment 49

[Francesco Lodolo [:flod]]

Attachment: Result with the attached logo

@pcheng
See comment 45

Comment 50

[Peipei Cheng (needinfo if you need my action)]

(In reply to pcheng from comment #48)
> As I know the favicon for Google/wikipedia is also 32*32. And they can be
> shown correctly. Does anyone know why this is different?

I see the reason now. Again, thanks a lot for your help! :P

Comment 51

[Axel Hecht [:Pike]]

Comment on attachment 773830 [details] [diff] [review]
Change Baidu's icon

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

r=me. Thanks for creating all those test cases, without looking at the explicit urls with one working, one not, I wouldn't have spotted that. Despite that it's just a week since I talked about it.

Comment 52

[Francesco Lodolo [:flod]]

No problem. Should I also land this? I currently don't see it on l10n-central, just on mozilla-aurora.

Comment 53

[Axel Hecht [:Pike]]

Yes, please. Don't bother about central, I think holywen does that every now and then anyway.

Comment 54

[Francesco Lodolo [:flod]]

Ok. Landed on mozilla-aurora
http://hg.mozilla.org/releases/l10n/mozilla-aurora/zh-CN/rev/742a7472f38a

Comment 55

[Tony Chung [:tchung]]

Verified fix on beta 10.  

cl.ly/image/0A013816172j