javaee / mojarra

PLEASE NOTE: This project has moved to Eclipse Foundation and will be archived under the JavaEE GitHub Organization. After Feb. 1, 2021, the new location will be github.com/javaee/mojarra. Mojarra - Oracle's implementation of the JavaServer Faces specification
https://github.com/eclipse-ee4j/mojarra
Other
164 stars 58 forks source link

SelectMenu's don't work with mixed type selectItems #446

Closed javaserverfaces closed 17 years ago

javaserverfaces commented 17 years ago

I have a class Entity with 2 subclasses (SubEntity1, SubEntity2).

I have a managed bean with a property of type Entity and a default value of an object of type SubEntity1.

I have a SelectOneMenu with with a number of SelectItems of mixed type SubEntity1 and SubEntity2.

When the renderer renders the options it attempts to match the available options with the selected options so it knows which one to mark as selected. When doing so MenuRenderer.renderOption() attempts to convert the candidate options to the type of the currently selected item. Where I have candidate options of both type SubEntity1 and SubEntity2 I get an EL Unable to Coerse Type exception when it attempts to coerse the candidate option of type SubEntity2 to the type of the selected item SubEntity1.

If the coercion is needed it would seem better to coerce it to the type of the managed bean's property instead of the type of the currently selected item. If the coercion is not necissary then we simply remove the coercion. Any opinions between those 2 options?

Mike

Environment

Operating System: All Platform: All

Affected Versions

[1.2_02]

javaserverfaces commented 17 years ago

Reported by youngm

javaserverfaces commented 17 years ago

@rlubke said: Discussed this issue briefly with Mike. We're think we can get the 'value' ValueExpression and obtain the property type and use that for the conversion class type instead of the selected value. If no VE is found, or the type is null, then default back to using the value type.

This should allow Mike's case to work.

However, it's late in the game for 1.2_03, and Mike has a workaround. We'll revisit for 1.2_04.

javaserverfaces commented 17 years ago

@rlubke said: Attempt to re-evaluate for 1.2_03.

javaserverfaces commented 17 years ago

@rlubke said: Created an attachment (id=394) Possible fix for SelectOne

javaserverfaces commented 17 years ago

@rlubke said: Created an attachment (id=395) Reproduction

javaserverfaces commented 17 years ago

@rlubke said: Reproduction URL: http://localhost:8080/WebStuff/faces/SelectOneTest.jsp

javaserverfaces commented 17 years ago

@rlubke said: Created an attachment (id=396) Build including the possible fix

javaserverfaces commented 17 years ago

@rlubke said: Mike,

I've attached a possible fix for UISelectOne, including the impl with the changes.

When you find time, please review and test.

javaserverfaces commented 17 years ago

youngm said: The fix you provided appears to work for me in the case I spelled out in the issue so r=youngm. The case below works with this patch where in rc3 it would throw an exception.

One interesting future note for the spec team though might be the situation a converter isn't used. If I don't use a converter then setValue(Object value) recieves a value of type "String" when I submit the Long item instead a Long.

I think a spec issue for more explicit consideration of mixed type selectItems would be a good spec issue.

Case: public Object getAsObject(FacesContext context, UIComponent component, String value) { if(NumberUtils.isNumber(value))

{ return new Long(value); }

return value; }

public String getAsString(FacesContext context, UIComponent component, Object value)

{ return value.toString(); }

public List getMixedTypeItems()

{ List list = new ArrayList(); list.add(new SelectItem("TestString")); list.add(new SelectItem(new Long(1000))); return list; }

public Object getValue()

{ return new Long(1000); }

public void setValue(Object value)

{ System.out.println(value.getClass()); }

javaserverfaces commented 17 years ago

youngm said: It's also unfortunate that we have to handle this mixed case with an exception being thrown....but I see the need to help ensure backwards compatibility. Perhaps in the next version of the el an coerse type check can be added to the api so an exception won't need to be caught for such situations.

javaserverfaces commented 17 years ago

@rlubke said: Created an attachment (id=398) Proposed Fix (version 2)

javaserverfaces commented 17 years ago

@rlubke said: Over-anaylyzed this one. I've attached a new change bundle that corrects the issue for selectone and selectmany.

Review pending.

javaserverfaces commented 17 years ago

jdlee said: Confusing typo in comments fixed by Ryan. No need for another CB for that.

r=jdlee

javaserverfaces commented 17 years ago

@rlubke said: Discussed the mixed options (without a common type) issue further with Mike. Doing so would require using a converter for each item which the spec currently doesn't allow for in the default UISelectMany case. Additionally, if using a List for the model type to collect the items, the type is considered String so converters aren't used.

Given this limitation, we'll look into creating a sandbox component that will override MenuRenderer/UISelectMany as necessary to support this.

javaserverfaces commented 17 years ago

@rlubke said: Fix checked in.

javaserverfaces commented 12 years ago

@manfredriem said: Closing issue out

javaserverfaces commented 17 years ago

File: changebundle.txt Attached By: @rlubke

javaserverfaces commented 17 years ago

File: changebundle.txt Attached By: @rlubke

javaserverfaces commented 17 years ago

File: jsf.zip Attached By: @rlubke

javaserverfaces commented 17 years ago

File: WebStuff.war Attached By: @rlubke

javaserverfaces commented 17 years ago

Was assigned to rlubke

javaserverfaces commented 7 years ago

This issue was imported from java.net JIRA JAVASERVERFACES-442

javaserverfaces commented 17 years ago

Marked as fixed on Monday, November 20th 2006, 2:11:06 am