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)
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)
679 bytes,
patch
|
Gavin
:
review+
mconnor
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•18 years ago
|
||
Assignee | ||
Updated•18 years ago
|
Attachment #228407 -
Flags: review? → review?(gavin.sharp)
Comment 2•18 years ago
|
||
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+
Assignee | ||
Comment 3•18 years ago
|
||
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
Comment 4•18 years ago
|
||
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.
Assignee | ||
Comment 5•18 years ago
|
||
(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.
Updated•18 years ago
|
Flags: blocking-firefox2? → blocking-firefox2+
Comment 6•18 years ago
|
||
(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. >
Comment 7•18 years ago
|
||
(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.
Comment 8•18 years ago
|
||
(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]
Comment 9•18 years ago
|
||
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.
Comment 10•18 years ago
|
||
(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.
Comment 11•18 years ago
|
||
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.
Comment 12•18 years ago
|
||
*** Bug 344650 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 13•18 years ago
|
||
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?
Updated•18 years ago
|
Attachment #228407 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 14•18 years ago
|
||
Fixed on both trunk and branch.
Comment 15•18 years ago
|
||
We need the hl= param for Google Suggest. The results below are different. http://suggestqueries.google.com/complete/search?output=firefox&client=firefox&qu=Fire http://suggestqueries.google.com/complete/search?output=firefox&client=firefox&qu=Fire&hl=ja
Comment 16•18 years ago
|
||
My locale is Japanese (ja and ja-JP-mac). How about other languages?
Comment 17•18 years ago
|
||
I think that you should file a new bug only for google suggestion case.
Comment 18•18 years ago
|
||
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
Comment 19•18 years ago
|
||
The same for german builds, did you already file a new bug, Kohei?
Comment 20•18 years ago
|
||
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.
Description
•