Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
chaloma1 opened this issue Mar 15, 2024 · 16 comments
Milestone

Comments

@chaloma1
Copy link

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
Copy link
Contributor

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

@chaloma1
Copy link
Author

chaloma1 commented Mar 15, 2024

@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
Copy link
Contributor

Just to be clear its a bug in MyFaces too?

@chaloma1
Copy link
Author

chaloma1 commented Mar 15, 2024

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
Copy link
Contributor

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

@chaloma1
Copy link
Author

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

@chaloma1
Copy link
Author

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

@BalusC
Copy link
Contributor

BalusC commented Mar 16, 2024

Again the EL literal vs VE matter.

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

BalusC added a commit that referenced this issue Mar 16, 2024
literal instead of VE, this will fail when the final value itself is VE
@BalusC
Copy link
Contributor

BalusC commented Mar 16, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

BalusC commented Mar 20, 2024

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.

BalusC added a commit that referenced this issue Mar 24, 2024
#5417: remove "optimization" when composite attribute default is literal instead of VE, this will fail when the final value itself is VE
@BalusC BalusC added this to the 4.0.7 milestone Mar 24, 2024
@BalusC BalusC closed this as completed Mar 24, 2024
BalusC added a commit that referenced this issue Mar 24, 2024
BalusC added a commit that referenced this issue Mar 24, 2024
pizzi80 added a commit to pizzi80/mojarra that referenced this issue Mar 25, 2024
+ eclipse-ee4j#5417: remove "optimization" when composite attribute default is literal instead of VE, this will fail when the final value itself is VE

Signed-off-by: pizzi80 <paolo@given2.com>
BalusC added a commit that referenced this issue Jul 27, 2024
attribute default value wasn't anymore available when explicitly calling
getAttributes().get(name) rather than relying on statehelper -- this has
been fixed
BalusC added a commit that referenced this issue Aug 3, 2024
@pktripathy
Copy link

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
Copy link
Contributor

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

@BalusC
Copy link
Contributor

BalusC commented Aug 26, 2024

Mentioned regression has been fixed as per #5460

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants