eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
158 stars 107 forks source link

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

Closed chaloma1 closed 3 months ago

chaloma1 commented 3 months ago

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 3 months ago

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

chaloma1 commented 3 months ago

@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 3 months ago

Just to be clear its a bug in MyFaces too?

chaloma1 commented 3 months ago

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

melloware commented 3 months ago

OK can you open a MyFaces ticket too please: https://issues.apache.org/jira/projects/MYFACES/issues/MYFACES-4655?filter=allopenissues

chaloma1 commented 3 months ago

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

chaloma1 commented 3 months ago

Issue at MyFaces created https://issues.apache.org/jira/browse/MYFACES-4656.

BalusC commented 3 months ago

Again the EL literal vs VE matter.

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

BalusC commented 3 months ago

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 3 months ago

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.

chaloma1 commented 3 months ago

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 3 months ago

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 3 months ago

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.