You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by Youngho Cho <yo...@nannet.co.kr> on 2002/08/14 03:13:53 UTC
[PATCH] DefaultTurbineRunData.java
Hello,
If I set my browser locale as ko and connect to Turbine-3 at first time
the defaultcharset was set EUC-KR.
( when I didn't set locale.default.charset )
and than when I connect to Turbine-3 at second time
using another browser which locale set as en_US
getCharSet() returns EUC-KR because the defaultCharSet was set as EUC-KR.
Therefore
getCharSet() doesn't return as per request property.
It returns the first hit browser setting value or defaultCharSet set up value.
So I would like to propose the below patch to give per request property to getCharSet() method.
Index: DefaultTurbineRunData.java
===================================================================
RCS file: /home/cvspublic/jakarta-turbine-3/src/java/org/apache/turbine/services/rundata/DefaultTurbineRunData.java,v
retrieving revision 1.16
diff -u -r1.16 DefaultTurbineRunData.java
--- DefaultTurbineRunData.java 29 May 2002 23:58:27 -0000 1.16
+++ DefaultTurbineRunData.java 14 Aug 2002 00:58:55 -0000
@@ -844,11 +844,7 @@
{
if (charSet == null)
{
- charSet = getDefaultCharSet();
- if (charSet == null)
- {
- charSet = TurbineMimeTypes.getCharSet(getLocale());
- }
+ charSet = TurbineMimeTypes.getCharSet(getLocale());
}
return charSet;
}
Thanks.
youngho
Re: [PATCH] DefaultTurbineRunData.java
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Jon Scott Stevens <jo...@latchkey.com> writes:
>on 8/13/02 6:13 PM, "Youngho Cho" <yo...@nannet.co.kr> wrote:
>> Hello,
>Also, your patch is bad...IMHO, the contents should really just be
>reversed...
>@@ -844,10 +845,10 @@
> {
> if (charSet == null)
> {
>- charSet = getDefaultCharSet();
>+ charSet = TurbineMimeTypes.getCharSet(getLocale());
> if (charSet == null)
> {
>- charSet = TurbineMimeTypes.getCharSet(getLocale());
>+ charSet = getDefaultCharSet();
> }
> }
> return charSet;
>Comments?
This makes much more sense.
Regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH hps@intermeta.de
Am Schwabachgrund 22 Fon.: 09131 / 50654-0 info@intermeta.de
D-91054 Buckenhof Fax.: 09131 / 50654-20
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [PATCH] DefaultTurbineRunData.java
Posted by Ilkka Priha <ip...@surfeu.fi>.
Youngho Cho wrote:
> Hello
>
>
>>So my suggestion is to leave the code as it is and configure the desired
>>behavior by modifying "charset.properties" and "turbine.properties".
>>
>>
> The current code doesn't have per request property as I reported
> even if the request have specific locale infomation
> whatever locale.default.charset , locale.default.language etc. set.
>
> Using jon's code
>
>>>@@ -844,10 +845,10 @@
>>> {
>>> if (charSet == null)
>>> {
>>>- charSet = getDefaultCharSet();
>>>+ charSet = TurbineMimeTypes.getCharSet(getLocale());
>>> if (charSet == null)
>>> {
>>>- charSet = TurbineMimeTypes.getCharSet(getLocale());
>>>+ charSet = getDefaultCharSet();
>>> }
>>> }
>>> return charSet;
>>>
>
> It give per request property.
> And if we want to fix a single charSet such as ISO-8859-1 ,
> than we should set all locale charSet as ISO-8859-1 in charset.properties
>
> But I think that it not good also.
> Because TurbineMimeTypes Service can use another modules except RunData.
>
> Thanks.
>
> youngho
Sorry Youngho,
I'm still living in the turbine-2 world, where the code doesn't set the
defaultCharSet based on the locale of the first request.
-- Ilkka
protected String getDefaultCharSet()
{
if (!defaultCharSetChecked)
{
/* Get the default charset and cache it in a static variable. */
defaultCharSet = TurbineResources.
getString("locale.default.charset");
defaultCharSetChecked = true;
}
String charset = defaultCharSet;
if (charset == null)
{
/* Default charset isn't specified, get the locale specific one. */
Locale locale = this.locale;
if (locale == null)
{
locale = getDefaultLocale();
}
if ((locale != null) && !locale.equals(Locale.US))
{
charset = TurbineMimeTypes.getCharSet(locale);
}
}
return charset;
}
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [PATCH] DefaultTurbineRunData.java
Posted by Youngho Cho <yo...@nannet.co.kr>.
Hello
>
> So my suggestion is to leave the code as it is and configure the desired
> behavior by modifying "charset.properties" and "turbine.properties".
>
The current code doesn't have per request property as I reported
even if the request have specific locale infomation
whatever locale.default.charset , locale.default.language etc. set.
Using jon's code
>>@@ -844,10 +845,10 @@
>> {
>> if (charSet == null)
>> {
>>- charSet = getDefaultCharSet();
>>+ charSet = TurbineMimeTypes.getCharSet(getLocale());
>> if (charSet == null)
>> {
>>- charSet = TurbineMimeTypes.getCharSet(getLocale());
>>+ charSet = getDefaultCharSet();
>> }
>> }
>> return charSet;
It give per request property.
And if we want to fix a single charSet such as ISO-8859-1 ,
than we should set all locale charSet as ISO-8859-1 in charset.properties
But I think that it not good also.
Because TurbineMimeTypes Service can use another modules except RunData.
Thanks.
youngho
Re: [PATCH] DefaultTurbineRunData.java
Posted by Ilkka Priha <ip...@surfeu.fi>.
Daniel Rall wrote:
>Jon Scott Stevens <jo...@latchkey.com> writes:
>
>>@@ -844,10 +845,10 @@
>> {
>> if (charSet == null)
>> {
>>- charSet = getDefaultCharSet();
>>+ charSet = TurbineMimeTypes.getCharSet(getLocale());
>> if (charSet == null)
>> {
>>- charSet = TurbineMimeTypes.getCharSet(getLocale());
>>+ charSet = getDefaultCharSet();
>> }
>> }
>> return charSet;
>>
>>Comments?
>>
>
>Jon, your suggested change is more correct than the original proposed
>change. I am not certain how it would affect things overall.
>
After the above change, the "locale.default.charset" property would not
have any effect. The MimeTypeService never returns null, but ISO-8859-1
if no other charset for a locale is found. On the other, there should
not be any reason to not to find a locale specific charset as all
relevant locales can be easily covered in "charset.properties".
The idea of the original (current) solution in the most typical scenario
is to specify a default locale with "locale.default.language" and
"locale.default.country" and leave "locale.default.charset" unspecified.
This configuration would check the locale specific charset for each
request and use the charset for the default locale if the request
doesn't specify a locale.
The purpose of the "locale.default.charset" is to disable this behavior
to save some processing time when the application is meant to support
only one locale, typically en_US. Another purpose is to specify a
generic encoding, typically UTF-8, for operator specific mobile services
where the gateway requires one.
So my suggestion is to leave the code as it is and configure the desired
behavior by modifying "charset.properties" and "turbine.properties".
-- Ilkka
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [PATCH] DefaultTurbineRunData.java
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Jon Scott Stevens <jo...@latchkey.com> writes:
> @@ -844,10 +845,10 @@
> {
> if (charSet == null)
> {
> - charSet = getDefaultCharSet();
> + charSet = TurbineMimeTypes.getCharSet(getLocale());
> if (charSet == null)
> {
> - charSet = TurbineMimeTypes.getCharSet(getLocale());
> + charSet = getDefaultCharSet();
> }
> }
> return charSet;
>
> Comments?
Jon, your suggested change is more correct than the original proposed
change. I am not certain how it would affect things overall.
--
Daniel Rall <dl...@finemaltcoding.com>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: [PATCH] DefaultTurbineRunData.java
Posted by Youngho Cho <yo...@nannet.co.kr>.
Hello Jon,
> I have seen similar behavior as you describe above and that is bad, but when
> I read the javadoc for the getCharSet() method, it points out a problem
> where 8859_1 might always want to be set regardless of what the browser
> sends. I'm not really sure how to deal with this properly. Any suggestions?
When I sent my patch , I thought that that kind of property is not RunData's
responsibility. But after read and think again, I agree.
Here is my second patch.
In order to satisfy all of us , I think we need to introduce a private static boolean charSetFixed flag
But this value check only one time!.
By default , getCharSet() returns defaultCharSet whether default.locale.charset or not
if default.locale.charset set than that value returns.
But if default.locale.charset.fixed set as false,
Than getCharSet() returns by request.
So most users use by default without any change or extra add ,
but some users who need extra property that getCharSet() returns by request
should be add default.locale.charset.fixed=false
Thanks,
youngho
Index: DefaultTurbineRunData.java
===================================================================
RCS file: /home/cvspublic/jakarta-turbine-3/src/java/org/apache/turbine/services/rundata/DefaultTurbineRunData.java,v
retrieving revision 1.16
diff -u -r1.16 DefaultTurbineRunData.java
--- DefaultTurbineRunData.java 29 May 2002 23:58:27 -0000 1.16
+++ DefaultTurbineRunData.java 15 Aug 2002 07:57:43 -0000
@@ -127,6 +127,11 @@
private static boolean defaultCharSetChecked;
/**
+ * The charset fixed flag.
+ */
+ private static boolean charSetFixed;
+
@@ -778,15 +783,18 @@
{
if (!defaultCharSetChecked)
{
- /* Get the default charset and cache
+ charSetFixed = Turbine.getConfiguration().getBoolean("locale.default.charset.fixed" , true );
+
+ /* Get the default charset and cache
it in a static variable. */
defaultCharSet = Turbine.getConfiguration().
getString("locale.default.charset");
if (defaultCharSet == null)
{
- /* Default charset isn't specified,
+ /* Default charset isn't specified,
get the locale specific one. */
+ /* is it should be getDefaultLocale() ? */
Locale locale = getLocale();
defaultCharSet = TurbineMimeTypes.getCharSet(locale);
}
@@ -844,11 +852,18 @@
{
if (charSet == null)
{
- charSet = getDefaultCharSet();
- if (charSet == null)
- {
- charSet = TurbineMimeTypes.getCharSet(getLocale());
- }
+ if( !defaultCharSetChecked )
+ {
+ getDefaultCharSet();
+ }
+
+ charSet = charSetFixed ? defaultCharSet : TurbineMimeTypes.getCharSet(getLocale());
+
+ if( charSet == null )
+ {
+ charSet = defaultCharSet;
+ }
+
}
return charSet;
}
Re: [PATCH] DefaultTurbineRunData.java
Posted by Jon Scott Stevens <jo...@latchkey.com>.
on 8/13/02 6:13 PM, "Youngho Cho" <yo...@nannet.co.kr> wrote:
> Hello,
>
> If I set my browser locale as ko and connect to Turbine-3 at first time
> the defaultcharset was set EUC-KR.
> ( when I didn't set locale.default.charset )
> and than when I connect to Turbine-3 at second time
> using another browser which locale set as en_US
>
> getCharSet() returns EUC-KR because the defaultCharSet was set as EUC-KR.
>
> Therefore
> getCharSet() doesn't return as per request property.
> It returns the first hit browser setting value or defaultCharSet set up value.
>
> So I would like to propose the below patch to give per request property to
> getCharSet() method.
This is interesting because there is a problem here.
I have seen similar behavior as you describe above and that is bad, but when
I read the javadoc for the getCharSet() method, it points out a problem
where 8859_1 might always want to be set regardless of what the browser
sends. I'm not really sure how to deal with this properly. Any suggestions?
Also, your patch is bad...IMHO, the contents should really just be
reversed...
@@ -844,10 +845,10 @@
{
if (charSet == null)
{
- charSet = getDefaultCharSet();
+ charSet = TurbineMimeTypes.getCharSet(getLocale());
if (charSet == null)
{
- charSet = TurbineMimeTypes.getCharSet(getLocale());
+ charSet = getDefaultCharSet();
}
}
return charSet;
Comments?
-jon
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>