Closed Bug 343849 Opened 18 years ago Closed 18 years ago

hl={moz:locale} in google plugin doesn't do the right thing

Categories

(Firefox :: Search, defect)

2.0 Branch
defect
Not set
blocker

Tracking

()

RESOLVED FIXED
Firefox 2 beta1

People

(Reporter: Pike, Assigned: Pike)

References

()

Details

(Keywords: fixed1.8.1, intl, regression)

Attachments

(1 file, 1 obsolete file)

I see no way to reliably do the hl= param, and I thought we found it rather unneeded, too, as google plays nicely with accept_lang.
This param was introduced as part of bug 335460.

Could we just get rid of it? The locale code is in the rls key, anyway, if it's about tracking stuff.

We should get this in for b1, feedback for P1 locales is just going to be embarrassing, I'm afraid.
Assignee: nobody → l10n
Status: NEW → ASSIGNED
Attachment #228407 - Flags: review?
Attachment #228407 - Flags: review? → review?(gavin.sharp)
Comment on attachment 228407 [details] [diff] [review]
remove hl={moz:locale} from google.xml

r=me, because it was never sent before, and obviously doesn't work for some locales. Based on bug 337719 comment 5, and discussion with Basil, I was under the impression that Google wanted this param sent, though, so perhaps it would be a good idea to raise it with them.
Attachment #228407 - Flags: review?(gavin.sharp) → review+
We have 32 locales picking up the en-US google.xml search plugin.

Affected locales:

fy-NL, hy-AM, pa-IN, sv-SE

nr, nso, ss, tn, ts, ve are south african locales coming up in english, which may or may not come up in something better for south africa if we don't use hl=, as Xhulu and two others we have are actually supported. I expected this, SA is going to stress test everything we have.

As Gavin said, we didn't use hl for most plugins in 1.5, and I actually once got the message to remove it.
Thus marking regression and asking for blocking-firefox2.

If you want to check this out yourself, I posted the bunch of query URLs to http://wiki.mozilla.org/User_talk:AxelHecht for now, but I'll not preserve that as historic evidence, for future reference.
Flags: blocking-firefox2?
Keywords: regression
I also commented this issue at the Bug 335460 comment 19.

This bug also affects ja-JP-mac, es-AR and es-ES langpacks at least.
(In reply to comment #4)
> I also commented this issue at the Bug 335460 comment 19.
> 
> This bug also affects ja-JP-mac, es-AR and es-ES langpacks at least.
> 

Well, those locales currently use their own versions of the google plugin and can simply hardcode hl to a working langcode, they wouldn't really have to use {moz:locale}.

But yes, we should consolidate the google plugin further, and then they'd be affected.
Flags: blocking-firefox2? → blocking-firefox2+
(In reply to comment #5)
Seems that google wants "es" instead of es-AR or es-ES.
Without hl being valid, Google show the english default results page.
With valid hl, google.com show the options to select pages in that language and also from that country.
Without hl parameter, on es-AR, Google works as expected.
> (In reply to comment #4)
> > I also commented this issue at the Bug 335460 comment 19.
> > 
> > This bug also affects ja-JP-mac, es-AR and es-ES langpacks at least.
> > 
> 
> Well, those locales currently use their own versions of the google plugin and
> can simply hardcode hl to a working langcode, they wouldn't really have to use
> {moz:locale}.
> 
> But yes, we should consolidate the google plugin further, and then they'd be
> affected.
> 

(In reply to comment #6)
> (In reply to comment #5)
> Seems that google wants "es" instead of es-AR or es-ES.

So what you are saying is that we should only be passing the first 2 characters or the part before the '-' as hl to Google?

Seems to me that if that is what they want, they could fix this on their end by ignoring the '-xx' part.
(In reply to comment #7)
> So what you are saying is that we should only be passing the first 2 characters
> or the part before the '-' as hl to Google?

In that case, both of Chinese-Taiwan (zh-TW) and Chinese-China (zh-CN) return "hl=zh" and google shows English pages.

To solve this bug,
1. we want Google to support the following scheme.
  When Google recieves "hl=ab-CD" request,
    if Google has "hl=ab-CD" page, show "hl=ab-CD" page.
    else if Google has "hl=ab" page, show "hl=ab" page.
    else show "hl=en" page.
or
2. apply attachment 228407 [details] [diff] [review]
I have a doubt. Why you want to use *UI locale* for the locale of google contents? We should use the first item of accept languages for it. Because that is *content locale*. E.g., current code breaks on the environments of some nightly build testers who is not English speaker. Because we are using English nightly build for test and daily use. But the accept languages includes our native language. It is enough for us. Please don't use UI locale for the content locale.
(In reply to comment #9)
> I have a doubt. Why you want to use *UI locale* for the locale of google
> contents? We should use the first item of accept languages for it. Because that
> is *content locale*. E.g., current code breaks on the environments of some
> nightly build testers who is not English speaker. Because we are using English
> nightly build for test and daily use. But the accept languages includes our
> native language. It is enough for us. Please don't use UI locale for the
> content locale.
> 

Good point!  I would hope that is what Google would do by default since the accept languages is passed automatically with all requests.  So perhaps we should just not pass hl as suggested in the original report, which is what attachment 228407 [details] [diff] [review] implements.
The feedback from Google is that supplying the proper hl parameter (normally only the two-character language, except when that is ambiguous) is the most reliable way to ensure that the user sees the correct page.  But if we want to remove it, they will not protest.

Personally, I think that getting hl correct is more trouble than it's worth.  This would be better handled on Google's end by always accepting the full locale, but I think that is unlikely to happen.

As long as omitting hl does the right thing in the important cases -- and since Firefox 1.5 shipped that way, we think it does -- then we can go ahead and drop hl.
*** Bug 344650 has been marked as a duplicate of this bug. ***
Comment on attachment 228407 [details] [diff] [review]
remove hl={moz:locale} from google.xml

Removed the hl={moz:locale} param on the google plugin on the trunk. If we need to tweak this for a particular locale, we should just do it in that locale.
Requesting branch approval.
Attachment #228407 - Flags: approval1.8.1?
Attachment #228407 - Flags: approval1.8.1? → approval1.8.1+
Fixed on both trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Attached patch add hl= param for suggest (obsolete) — Splinter Review
My locale is Japanese (ja and ja-JP-mac). How about other languages?
I think that you should file a new bug only for google suggestion case.
Comment on attachment 235370 [details] [diff] [review]
add hl= param for suggest

Thanks, Nakano-san. I'll file another bug.

# Anyway {moz:locale} doesn't work in this case..
Attachment #235370 - Attachment is obsolete: true
The same for german builds, did you already file a new bug, Kohei?
I've noticed that the Google Suggest supports only hl=en and hl=ja for now.
It may not work in other langs. Sorry for spam...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: