903084 - Add Bing as a general search provider for specified locales for Fennec

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Karen Rudnitski [:kar]]

Can we look at adding Bing as a general search provider for the following locales: en-US, en-GB, de, fr, es-ES.

(If we get en-CA miraculously in time, we would also want Bing included for that locale as well.)

*Please let me know which are actually used, and whether this is enough information to go upon or if more info from Bing is required.*

Bing mobile form code = MOZMBA  We just switch out MOZSBR in the search string (i.e. http://www.bing.com/search?q=whatever&pc=MOZI&form=MOZSBR

I would like to see Bing simply added to our list so that it falls below 'Google'.
  ie for en-US:
     Google
     Bing
     <Yahoo - bug 903082>
     Amazon
     Twitter
     Wikipedia

Please test for both phones and tablets.

The release target is Fx 26, although if this has any possibility of being pulled into Fx 25, do let me know as I want to keep Bing informed.

Please ensure the communities for the affected locales are informed and if there are any serious concerns raised as a result of this change.

Thanks, Karen

Comment 2

[Richard Newman [:rnewman]]

Engine identifier = "bing", matching desktop?

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

This is tracking Fennec 25 and week 2 just wrapped up. The window for this landing in 25 beta is rapidly closing especially with the Summit taking up part of a week. If this should not be tracking 25 please renominate so we can retriage.

Comment 4

[Alex Keybl [:akeybl]]

Landing this week, 26 and up only

Comment 5

[Karen Rudnitski [:kar]]

Attachment: bing.patch

This is the patch that adds Bing as a search provider to en-US only. It is not set as default. It does not have search suggestions, tracking information and the icon is pretty poor - we have requested information for all of these areas from Bing.

Ideally this would be set as position 3, but another bug has been logged to allow us to set that position (bug 923798). I will need to modify when this bug has been fixed.

Comment 6

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

Comment on attachment 813865 [details] [diff] [review]
bing.patch

Where did you get info on the "pc" and "form" params? I see different ones in the desktop definition:
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/searchplugins/bing.xml

The desktop definition also has search suggestions.

Comment 7

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

Can you update this patch:
Add a "3" here using the same pattern:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/app/mobile.js#247

Add a "3" here with the "Bing":
http://mxr.mozilla.org/mozilla-central/source/mobile/locales/en-US/chrome/region.properties#9

Comment 8

[Karen Rudnitski [:kar]]

(In reply to Mark Finkle (:mfinkle) from comment #6)

> Where did you get info on the "pc" and "form" params? 

I got them direct from Bing to use for our Firefox for Android implementation. I have already gone back to ask them about search suggestions, reporting and possibly different icon.

Updated patch to follow soon as per comment 7.

Comment 9

[Karen Rudnitski [:kar]]

Attachment: bing patch 2

Adding Bing as the third position

Comment 10

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

Comment on attachment 813936 [details] [diff] [review]
bing patch 2

Looks good, but you forgot to make the "3" addition here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/app/mobile.js#247

You'll need that so the "3" line in region.properties is used

Comment 11

[Aaron Train [:aaronmt]]

Please see my comment in the Yahoo bug (bug 903082) that may have the same affect on the proposed patch here.

Comment 12

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #10)
> Comment on attachment 813936 [details] [diff] [review]
> bing patch 2
> 
> Looks good, but you forgot to make the "3" addition here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/app/mobile.
> js#247
> 
> You'll need that so the "3" line in region.properties is used

I think doing that will also resolve bug 923798

Comment 13

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

(In reply to Aaron Train [:aaronmt] from comment #11)
> Please see my comment in the Yahoo bug (bug 903082) that may have the same
> affect on the proposed patch here.

Looks like we use {searchTerms} here, so this patch is good.

That said, I am fixing this patch and will add the Yahoo fix here

Comment 14

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

Attachment: Karen's patch with bing tweak and yahoo fix

This builds on Karen's patch. It adds the missing mobile.js entry and adds the {searchTerms} fix for Yahoo.

* Yahoo and Bing are in the 2 and 3 positions
* Yahoo and Bing both search for the right terms

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bf856e026871

Comment 16

[Erin Lancaster [:elan]]

Changing this target milestone to 26. We want to uplifted...

Comment 17

[Richard Newman [:rnewman]]

Comment on attachment 814375 [details] [diff] [review]
Karen's patch with bing tweak and yahoo fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  New feature request: adding search engine to default set.

User impact if declined: 
  Fewer search engines!

Testing completed (on m-c, etc.): 
  Baking on Nightly for a while.

Risk to taking this patch (and alternatives if risky): 
  Low: just adding plugins to our searchplugin list.

String or IDL/UUID changes made by this patch:
  Adding a param to the Yahoo search plugin, which is a per-locale thing, and defining a new search engine. Both of these are scoped to /mobile/. As far as I know, l10n is in the loop for this…

Comment 18

[Richard Newman [:rnewman]]

Erin: Target Milestone indicates where this work first landed. We need to request approval and use the status flags to upstream the work, so I just did that.

Comment 19

[Erin Lancaster [:elan]]

Perfect, thank you.

Comment 20

[Francesco Lodolo [:flod]]

I'm slightly confused by this bug: it's marked as verified fixed but the subject says add Bing "for specific locales", not "for en-US".

Comment 21

[Karen Rudnitski [:kar]]

Ensuring this one is also on Jeff's radar to work through the other localizers

Comment 22

[Jeff Beatty [:gueroJeff]]

(In reply to Karen Rudnitski [:kar] from comment #21)
> Ensuring this one is also on Jeff's radar to work through the other
> localizers

On my radar. Please see comments 32 & 33 from bug 903082 in order to specify exactly what files need to be altered and landed in their repositories for this.

Comment 23

[Karen Rudnitski [:kar]]

Attachment: Bing Favicon 10-24-13.zip

Comment 24

[Karen Rudnitski [:kar]]

We have a new favicon from Bing with their new logo (attached) - can you please see if either work any better? The yellow 'b' on grey is their preferred option, with the grey 'b' on yellow is their second preference.

Comment 25

[Karen Rudnitski [:kar]]

(In reply to jbeatty from comment #22)
> (In reply to Karen Rudnitski [:kar] from comment #21)
> > Ensuring this one is also on Jeff's radar to work through the other
> > localizers
> 
> On my radar. Please see comments 32 & 33 from bug 903082 in order to specify
> exactly what files need to be altered and landed in their repositories for
> this.

Jeff, we're making an educated guess while the confirmations are going out. We believe Bing should redirect, therefore no change to xml files should be required. Can we therefore proactively get this started from your end? No work should be wasted regardless.

Comment 26

[Francesco Lodolo [:flod]]

(In reply to Karen Rudnitski [:kar] from comment #25)
> Jeff, we're making an educated guess while the confirmations are going out.
> We believe Bing should redirect, therefore no change to xml files should be
> required. Can we therefore proactively get this started from your end? No
> work should be wasted regardless.

Automatic redirects based on locale -> list.txt will contain references to en-US searchplugins (pretty quick to do)
No redirects -> we need local .xml files and completely different values in list.txt (need more times)

So we actually need to know which one of two situation we're in before starting.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Carrying over the aurora approval from prior to the uplift (plus this blocks bug 903082 and bug 923795 which both have beta approval).

https://hg.mozilla.org/releases/mozilla-beta/rev/e1718472289d

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Automated merge from beta to b2g26.
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/e1718472289d

Comment 29

[Mike Connor [:mconnor]]

Is there a followup I can't find for adding suggestions to Bing?

Comment 30

[Karen Rudnitski [:kar]]

I'm on an email thread right now with Bing to get to the bottom of search suggestions (via internal contacts to try and finally get some traction)

Comment 31

[Karen Rudnitski [:kar]]

Ok, search experts. I have the following string for Bing search suggestions, where the FORM needs to equal ours (from comment 0: MOZMBA).

http://api.bing.com/qsonhs.aspx?FORM=ASAPIH&mkt=en-GB&type=cb&cb=sa_inst.apiCB&q=jennifer&cp=8&sid=24B5BFD2BA0D4F4B8FEEF1DF76B521A9&qt=1386945894943&bq=jennifer&count=12&o=p+hs

HOWEVER, before this is coded in, an someone please tell me whether this makes sense? I see they have defined a market and also has a reference to 'jennifer'. I would therefore like someone who knows more about this than me to tell me whether this makes sense or a search suggestion string or not.

Thanks and most appreciated as always,
Karen

Comment 32

[Richard Newman [:rnewman]]

(In reply to Karen Rudnitski [:kar] from comment #31)

The only open questions to me are:

* Do we need to include that timestamp (qt), and
* Do we need to include the search twice, and
* Is the sid something that differs per search, or is it our API ID?

Something like this should be the outcome:

    <Url type="application/x-suggestions+json" template="http://api.bing.com/qsonhs.aspx">
        <Param name="mkt" value="{moz:locale}"/>
        <Param name="type" value="cb"/>
        <Param name="cp" value="8"/>
        <Param name="count" value="12"/>
        <Param name="o" value="p+hs"/>
        <Param name="bq" value="{searchTerms}"/>
        <Param name="q" value="{searchTerms}"/>
        <Param name="FORM" value="ASAPIH"/>
    </Url>

Comment 33

[Richard Newman [:rnewman]]

Here, for contrast, is desktop's:

    <Url type="application/x-suggestions+json" template="http://api.bing.com/osjson.aspx">
        <Param name="query" value="{searchTerms}"/>
        <Param name="form" value="OSDJAS"/>
    </Url>

Comment 34

[Mike Connor [:mconnor]]

That URL is wrong, and won't work (wrong format.  I've got an email out to Bing to clarify and resolve the correct URL, which should look pretty close to the desktop URL (which uses a different format).

Comment 35

[Mike Connor [:mconnor]]

I've spun the suggestions issue off to bug 950201 as a clone of this (so everyone should be CCed).