Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: EE-3.0.0.GA, 3.1
-
Fix Version/s: EE-3.0.0.GA_P01, 3.2
-
Component/s: Framework
-
Labels:None
-
Environment:N/A
Description
Currently DomBasicRenderer.java class does not support detection of global String converters in an application. It is designed in such way that it always returns a String value before detecting a global custom String converter. (line 205@ DomBasicRenderer.java)
201 if (converter == null) {
202 if (currentValue == null) {
203 return "";
204 } else if (currentValue instanceof String) {
205 return (String) currentValue;
206 }
207
208 converter = getConverterForClass(currentValue.getClass());
209
210 if (converter == null) {
211 return currentValue.toString();
212 }
213 }
a fix below is proposed by one of our clients to allow detection of such converters in application:
...
if (converter == null) {
if (currentValue == null) {
return "";
} else if (currentValue instanceof String) {
// Addition for global String converter detection.
converter = getConverterForClass(currentValue.getClass());
if (converter == null) {
return (String) currentValue;
} else {
return converter.getAsString(facesContext, uiComponent, currentValue);
}
}
converter = getConverterForClass(currentValue.getClass());
if (converter == null) {
return currentValue.toString();
}
}
...
We need to review proposed fix and determine if it can be safely incorporated into our code.
201 if (converter == null) {
202 if (currentValue == null) {
203 return "";
204 } else if (currentValue instanceof String) {
205 return (String) currentValue;
206 }
207
208 converter = getConverterForClass(currentValue.getClass());
209
210 if (converter == null) {
211 return currentValue.toString();
212 }
213 }
a fix below is proposed by one of our clients to allow detection of such converters in application:
...
if (converter == null) {
if (currentValue == null) {
return "";
} else if (currentValue instanceof String) {
// Addition for global String converter detection.
converter = getConverterForClass(currentValue.getClass());
if (converter == null) {
return (String) currentValue;
} else {
return converter.getAsString(facesContext, uiComponent, currentValue);
}
}
converter = getConverterForClass(currentValue.getClass());
if (converter == null) {
return currentValue.toString();
}
}
...
We need to review proposed fix and determine if it can be safely incorporated into our code.
Activity
- All
- Comments
- History
- Activity
- Remote Attachments
- Subversion
I've checked the modification into the branch and the main trunk for regression testing. We can either wait for the test case from the customer to exercise the code or we can potentially provide them with a patch or test branch for them to test.
Show
Deryk Sinotte
added a comment - I've checked the modification into the branch and the main trunk for regression testing. We can either wait for the test case from the customer to exercise the code or we can potentially provide them with a patch or test branch for them to test.
I reviewed the change and the relevant section of the code. It looks reasonable to me. I changed the logic a bit to make it easier to follow and get rid of the repetitive bits:
197
{ 203 return converter.getAsString(facesContext, uiComponent, currentValue); 204 }198 // look to see whether there is a converter registered with the component
199 Converter converter = ((ValueHolder) uiComponent).getConverter();
200
201 // if there is a converter, then use it
202 if( converter != null )
{ 212 ret = converter.getAsString(facesContext, uiComponent, currentValue); 213 }205
206 // if there was no converter registered with the component then
207 // look for the converter associated with the class of the currentValue
208 String ret = "";
209 if (currentValue != null) {
210 converter = getConverterForClass(currentValue.getClass());
211 if (converter != null)
else
{ 214 ret = currentValue.toString(); 215 }216 }
217 return ret;
218 }
I'll await the test case before committing to branch and trunk?