eclipse-ee4j/mojarra

Component default required attribute is evaluated differently with xhtml/java implementation

Closed this issue · 16 comments

chaloma1 commented

Describe the bug

  1. Component with required attribute and default parameter specified in xhtml interface definition ignores calling java methods in xhtml like #{true} or #{xxxController.isComponentRequired}. If the required attribute is set as plain string "true", then it works.

  2. Component with not defined default value in xhtml works with both #{true}, #{xxxController.isComponentRequired} or plain string "true"

  3. Component with overriden isRequired() method using "required" instead PropertyKeys.required string as in UIInput parent class (you can add default value here if attribute is not found or set). Works with #{true} or #{xxxController.isComponentRequired} but not with plain string like "true".

To Reproduce

primefaces-test-git.zip
See the attached sample. You can execute the sample with mvn jetty:run command and hit http://localhost:8080/primefaces-test to run the page.

Expected behavior

Implementation type should not matter (at least the required attribute within xhtml component interface) and all of those values should have the same required result.

In the sample project i also tried using non-component specific attribute for FirstComponent like "styleClass" which is working ok even with default value.

Environment

I used primefaces test project to reproduce the issue. Sample is using Mojarra 4.0.5 version.

melloware commented

@chaloma1 does the same happen with MyFaces 4? mvn clean jetty:run -Pmyfaces40 ?

chaloma1 commented

@melloware tried with myFaces:

FirstComponent which has required default value in xhtml works ok.
SecondComponent is just for show without default, so no difference here.
ThirdComponent which has default value set by overriden isRequired method works the same. (so plain string doesnt work)

melloware commented

Just to be clear its a bug in MyFaces too?

chaloma1 commented

Default set in xhtml (component interface) is fixed in MyFaces -> so this seems like a bug in Mojarra only
Default set by overriding isRequired method in Component class works the same in MyFaces as in Mojarra (which means it does not recognize plain string value -> required="true" while inserting component to a page) -> bug in Mojarra and MyFaces as well

chaloma1 commented

Sure, just need few days until new Jira account is created.

BalusC commented

Again the EL literal vs VE matter.

Work around for now, use default="#{false}" instead of default="false".

BalusC commented

By the way, thisSystem.out.println in your test project makes no sense:

  public FirstComponent() {
    super();
    System.out.println("First component: " + this.getClientId() + " " + this.isRequired());
  }

The attributes/properties can't be set before construction. It goes like this:

FirstComponent component = new FirstComponent();
component.setId(...);
component.setRequired(...);

and thus not like this ;)

FirstComponent component;
component.setId(...);
component.setRequired(...);
component = new FirstComponent();
chaloma1 commented

By the way, thisSystem.out.println in your test project makes no sense

Tried multiple things because i was not sure how the default parameter was applied at that time. But yes, you are correct, thanks.

  • 👍1
chaloma1 commented

Work around for now, use default="#{false}" instead of default="false".

Works, thanks.

The ThirdComponent with overriden isRequired method is still a mystery though.

chaloma1 commented

Third component problem with overriden isRequired method was solved in MyFaces ticket. Fault was on my side, when i was looking at the stateHelper eval function i did not notice that "required" isnt the same as PropertyKeys.required enum. The eval function looks for key.toString() only for the value expressions. So literal value was not found.

If anyone uses

@Override
public boolean isRequired() {
  return (Boolean) getStateHelper().eval("required", false);
}

then setRequired is needed as well

@Override
public void setRequired(boolean required) {
  getStateHelper().put("required", required );
}
BalusC commented

The override is unnecessary unless you want to perform additional action but then you should be doing something like:

@Override
public boolean isRequired() {
    boolean required = super.isRequired();
    yourAdditionalAction(required);
    return required;
}

If you aren't doing that, just remove the method. Its default behavior is fine.

  • 👍1
pktripathy commented

This fix has broken default value implementation in composite component. InstanceFactory.pushDeclaredDefaultValuesToAttributesMap no longer adds attributes to attrs, so default attributes which is not supplied in call to composite component is not available in attrs.

melloware commented

@pktripathy please open a new issue then with your details.

BalusC commented

Mentioned regression has been fixed as per #5460