352672 - May wanna add hl={moz:language} to google.xml's suggest

Status: VERIFIED FIXED

(Firefox :: Search, enhancement, P4)

Description

[Axel Hecht [:Pike]]

Right now, google offers to corpuses, one global, one for Japan.

For 2.0, we're adding a hl=ja to a distinct google-ja.xml to select that corpus.

As google adds more corpuses, adding a hl={moz:language} to the generic google.xml should improve UE for more locales.

This should be developed in parallel, so that we can test the implementation on both sides. Once that works on the trunk, it might be a good idea to backport that support to the 1.8 branch, too.

As the hl param really only wants the language, we'll need to add support for that to nsSearchService.js.

Comment 1

[Axel Hecht [:Pike]]

This is a spin-off from bug 350177, btw.

Comment 2

[Uri Bernstein]

Yes, we definitely want this. Google Suggest currently supports several additional languages (zh-CN, zh-TW, ko), and might support others in the future.

(In reply to comment #0)
> As the hl param really only wants the language, we'll need to add support for
> that to nsSearchService.js.

Could you explain? hl can certainly include a locale, e.g. zh-CN or pt-BR (try searching on google.cn or google.com.br).

Comment 3

[Uri Bernstein]

Attachment: patch

This adds the hl parameter and also changes "qu" to "q" (which is the preferred name for this parameter).

Comment 4

[Uri Bernstein]

Attachment: patch

{moz:locale} is actually probably more correct than {language}.

Comment 5

[Uri Bernstein]

Attachment: patch, take 3

D'oh. I hope I got it right this time. Sorry for the spam.

Comment 6

[Axel Hecht [:Pike]]

moz:locale doesn't cut it in general, compare 

http://suggestqueries.google.com/complete/search?output=firefox&client=firefox&hl=ja-JP-mac&qu=t
and
http://suggestqueries.google.com/complete/search?output=firefox&client=firefox&hl=ja&qu=t

So, at least for our Japanese mac locale, we need special handling, and I take this as a sign that it might not work completely.

I wonder, does the hl= logic somehow correlate to accept-language headers? The latter might be really interesting in particular for minority languages.

Comment 7

[Uri Bernstein]

OK, I'll see what can be done about this on Google's side.

BTW, why do we have a "Japanese mac locale"? How is it different than the regular Mac locale?

Comment 8

[Axel Hecht [:Pike]]

The Mac uses a different set of computer language terms in Japan, IIRC.

Comment 9

[Uri Bernstein]

(In reply to comment #6)
> moz:locale doesn't cut it in general, compare 
> 
> http://suggestqueries.google.com/complete/search?output=firefox&client=firefox&hl=ja-JP-mac&qu=t
> and
> http://suggestqueries.google.com/complete/search?output=firefox&client=firefox&hl=ja&qu=t
> 
> So, at least for our Japanese mac locale, we need special handling, and I take
> this as a sign that it might not work completely.
> 

This has been fixed on the server side (e.g., the queries above now return the same results). The fix is general: whenever "ab-CD(...)" is not recognized, "ab" is used instead (assuming it's supported, of course. The fallback is English).  

Gavin - can you please review the patch now?

Comment 10

[Uri Bernstein]

I'd really like to have this for Fx 3.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Sorry for the delay here!

Comment 12

[Uri Bernstein]

Checked in:
Checking in mozilla/browser/locales/en-US/searchplugins/google.xml;
/cvsroot/mozilla/browser/locales/en-US/searchplugins/google.xml,v  <--  google.xml
new revision: 1.13; previous revision: 1.12
done

Comment 13

[Vlado Valastiak [:wladow]]

What about google plugins in /l10n. Do you plan a general patch or should each localization team update their plugin? 

Comment 14

[Axel Hecht [:Pike]]

Vlado, which google plugins? Note, for all the plugins that are called google.xml, we pick up the one in en-US by build magic. This should really only affect google-jp.xml, which already sends hl.

Comment 15

[Vlado Valastiak [:wladow]]

If so, this one http://lxr.mozilla.org/l10n/source/sk/browser/searchplugins/google.xml is obsolete and can be removed, right?

Comment 16

[Axel Hecht [:Pike]]

yes.

Comment 17

[Uri Bernstein]

(In reply to comment #15)
> If so, this one
> http://lxr.mozilla.org/l10n/source/sk/browser/searchplugins/google.xml is
> obsolete and can be removed, right?
> 

And the same applies to all the files listed in http://mxr.mozilla.org/l10n/find?string=google.xml , right?

Comment 18

[Axel Hecht [:Pike]]

Yes. I filed bug 411162 to track that.

Comment 19

[P. Rutgers]

verified fix in Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3