Closed
Bug 352672
Opened 18 years ago
Closed 17 years ago
May wanna add hl={moz:language} to google.xml's suggest
Categories
(Firefox :: Search, enhancement, P4)
Firefox
Search
Tracking
()
VERIFIED
FIXED
Firefox 3 beta3
People
(Reporter: Pike, Assigned: uriber)
Details
(Whiteboard: [has patch])
Attachments
(1 file, 2 obsolete files)
2.87 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•18 years ago
|
||
This is a spin-off from bug 350177, btw.
Assignee | ||
Comment 2•17 years ago
|
||
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).
Status: NEW → ASSIGNED
Version: 2.0 Branch → Trunk
Assignee | ||
Comment 3•17 years ago
|
||
This adds the hl parameter and also changes "qu" to "q" (which is the preferred name for this parameter).
Assignee: nobody → uriber
Attachment #283569 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #283569 -
Flags: review? → review?(gavin.sharp)
Assignee | ||
Comment 4•17 years ago
|
||
{moz:locale} is actually probably more correct than {language}.
Attachment #283569 -
Attachment is obsolete: true
Attachment #283571 -
Flags: review?(gavin.sharp)
Attachment #283569 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 5•17 years ago
|
||
D'oh. I hope I got it right this time. Sorry for the spam.
Attachment #283571 -
Attachment is obsolete: true
Attachment #283572 -
Flags: review?(gavin.sharp)
Attachment #283571 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 6•17 years ago
|
||
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.
Assignee | ||
Comment 7•17 years ago
|
||
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?
Reporter | ||
Comment 8•17 years ago
|
||
The Mac uses a different set of computer language terms in Japan, IIRC.
Assignee | ||
Comment 9•17 years ago
|
||
(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?
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P4
Whiteboard: [has patch][need review gavin]
Target Milestone: --- → Firefox 3 M11
Updated•17 years ago
|
Attachment #283572 -
Flags: review?(gavin.sharp) → review+
Comment 11•17 years ago
|
||
Sorry for the delay here!
Whiteboard: [has patch][need review gavin] → [has patch]
Assignee | ||
Comment 12•17 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 13•17 years ago
|
||
What about google plugins in /l10n. Do you plan a general patch or should each localization team update their plugin?
Reporter | ||
Comment 14•17 years ago
|
||
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•17 years ago
|
||
If so, this one http://lxr.mozilla.org/l10n/source/sk/browser/searchplugins/google.xml is obsolete and can be removed, right?
Reporter | ||
Comment 16•17 years ago
|
||
yes.
Assignee | ||
Comment 17•17 years ago
|
||
(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?
Reporter | ||
Comment 18•17 years ago
|
||
Yes. I filed bug 411162 to track that.
Comment 19•17 years ago
|
||
verified fix in Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•