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

<f:ajax> execute="@this" and render="@this" does not behave as expected when nested in composite component #1567

Closed
BalusC opened this issue Apr 1, 2021 · 17 comments
Milestone

Comments

@BalusC
Copy link
Member

BalusC commented Apr 1, 2021

Given:

<cc:interface ...>
    ...
    <cc:clientBehavior name="change" default="true" targets="hour minute" event="change" />
</cc:interface>
<cc:implementation>
    ...
    <h:selectOneMenu id="hour" />
    <h:selectOneMenu id="minute" />
    ...
</cc:implementation>

When:

<my:inputLocalTime>
    <f:ajax execute="@this" />
</my:inputLocalTime>

Then the expectation is that the "entire" <my:inputLocalTime> is executed (at least the components covered in targets attribute of the associated <cc:clientBehavior>). The current unintuitive behavior is that it's only executing the individual <h:selectOneMenu> component on which the event is triggered.

The same problem applies to render attribute.

When:

<my:inputLocalTime>
    <f:ajax render="@this" />
</my:inputLocalTime>

Then the expectation is that the entire <my:inputLocalTime> is rendered. The current unintuitive behavior is that it's only rendering the invididual <h:selectOneMenu> component on which the event is triggered.

@BalusC BalusC changed the title Faces 4.0: <f:ajax execute="@this"> should be Faces 4.0: <f:ajax> nested in any composite component should infer "targets" attribute of any <cc:clientBehavior default="true"> as default values for "execute" attribute Apr 1, 2021
@BalusC BalusC changed the title Faces 4.0: <f:ajax> nested in any composite component should infer "targets" attribute of any <cc:clientBehavior default="true"> as default values for "execute" attribute Faces 4.0: <f:ajax> execute="@this" and render="@this" does not behave as expected when nested in composite component Apr 11, 2021
@arjantijms arjantijms added the 4.0 label Apr 13, 2021
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Apr 14, 2021
@BalusC
Copy link
Member Author

BalusC commented Apr 14, 2021

@tandraschko: is this updated spec for you also sufficient to fix this issue in MyFaces side? eclipse-ee4j/mojarra@bc2d918

BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Apr 14, 2021
@tandraschko
Copy link

@BalusC which reported MF issue do you mean?

@BalusC
Copy link
Member Author

BalusC commented Apr 14, 2021

The current one.

@tandraschko
Copy link

yep, i think @this should reference to the parent CC in this case.
But im not sure how its currently implemented in both Mojarra and MyFaces?

@BalusC
Copy link
Member Author

BalusC commented Apr 14, 2021

The unexpected behavior is described in issue description. In Mojarra, it's because the f:ajax is basically re-attached to the individual components. So the @this is simply evaluated in their context.

@tandraschko
Copy link

i think its the same for MyFaces
Not sure how easy it is to change this behavior; do you already work on a PR for Mojarra?

@tandraschko
Copy link

NVM: pressed f5 ;D
i willl check it for MyFaces

BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Apr 14, 2021
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Apr 14, 2021
@BalusC
Copy link
Member Author

BalusC commented Apr 14, 2021

F5 again, initial impl did not exactly work as intended ;)

@BalusC
Copy link
Member Author

BalusC commented Apr 14, 2021

PR created. All local tests passed (although it's missing a good bunch of f:ajax tests since migration ..)

@BalusC
Copy link
Member Author

BalusC commented Apr 18, 2021

I've backported impl improvements to Mojarra 2.3 nonetheless. All f:ajax tests over there now pass.

@BalusC
Copy link
Member Author

BalusC commented Apr 19, 2021

Merged in 3.0 as well.

@BalusC BalusC closed this as completed Apr 19, 2021
@tandraschko
Copy link

@BalusC
i currently check this in MF and it seems that AjaxBehavior#setExecute is already called with the remapped clientId (button) insteadof @this
so we have to manipulate the execute string/list in the AjaxBehavior
is there any reason you didnt try to make this "remapping" earlier, so AjaxBehavior#setExecute is already called with the CC clientId?

Also i wonder if we can implement this a bit different, so that even p:ajax might work automatically?

@BalusC
Copy link
Member Author

BalusC commented Apr 19, 2021

No idea. Logic was already like this and I didn't want to break backwards compatibility. If I were to guess I think it's to ensure that any EL in execute/render attribute is evaluated at right moment.

@tandraschko tandraschko added this to the 4.0 milestone Dec 2, 2021
@codylerum
Copy link

@BalusC I'm throwing this out there as I'm seeing some behavior changes in mojarra going form 2.3.14 to 2.3.17 which seem to be related to this change. What I'm seeing is in the below where <oc:wizardSection> is a compositeComponent with no <cc:clientBehavior defined. It really is just a layout container.

What happens is when the h:selectOneMenu is changed and the f:ajax triggers it is executing all of the input fields in the <oc:wizardSection rather than just the <h:selectOneMenu This obviously can cause an issue when other fields which may be required=true are not yet set.

<oc:wizardSection section="#{emergencyAddressUpdateView.inputSection}" label="Address Input">
	<o:decorateInput label="Calling Name">
		<h:inputText id="name" required="true" value="#{emergencyAddressUpdateView.callingName}" p:autocomplete="new-name" maxlength="32" />
		<p class="form-text">Name sent to Emergency Services. Could be an individual or business name. This is seperate from Caller-Id</p>
	</o:decorateInput>
	<o:decorateInput label="Suggestions">
		<h:selectOneMenu id="suggestions" value="#{emergencyAddressUpdateView.selectedServiceAddress}" styleClass="form-control" converter="ServiceAddressConverter">
			<f:selectItem itemLabel="Choose" />
			<f:selectItems value="#{emergencyAddressUpdateView.serviceAddresses}" />
			<f:ajax execute="@this" render="@form" listener="#{emergencyAddressUpdateView.applyServiceAddress()}" />
		</h:selectOneMenu>
	</o:decorateInput>
</oc:wizardSection>

oc:wizardSection is a composite component
o:decorateInput is a custom component (not composite)

This did not occur in 2.3.14, but I'm not sure yet if this is the intent. My reading of this issue and the spec would lead me to believe the @this should only apply if it is a direct child of the composite like

<my:inputLocalTime>
    <f:ajax execute="@this" />
</my:inputLocalTime>

But probably shouldn't execute all inputs if it was like

<my:inputLocalTime>
  <h:inputText>
      <f:ajax execute="@this" />
  </<h:inputText>
</my:inputLocalTime>

If any <f:ajax execute="@this" /> contained within a composite (including it's inserted children as is the case here causes all inputs contained within the composite to be executed then composites can't really be used as layout containers.

@BalusC
Copy link
Member Author

BalusC commented Jan 10, 2022

But probably shouldn't execute all inputs if it was like

You're right. I'll have to take a second look at this.

@BalusC BalusC reopened this Jan 10, 2022
@arjantijms arjantijms removed this from the 4.0 milestone May 19, 2022
@tandraschko tandraschko added this to the 4.1 milestone Jun 9, 2022
@tandraschko tandraschko changed the title Faces 4.0: <f:ajax> execute="@this" and render="@this" does not behave as expected when nested in composite component <f:ajax> execute="@this" and render="@this" does not behave as expected when nested in composite component Jun 9, 2022
@tandraschko tandraschko removed the 4.0 label Jun 9, 2022
@BalusC
Copy link
Member Author

BalusC commented Jul 29, 2023

Reproduced. The spec was correct, it's actually a bug in Mojarra impl and happens only when a composite is doubly-nested.

@BalusC
Copy link
Member Author

BalusC commented Jul 30, 2023

Unsure why this has 4.1 tag/milestone. It's added in 4.0. See updated execute and render attributes of f:ajax: https://jakarta.ee/specifications/faces/4.0/vdldoc/f/ajax.html

I should probably not have reopened this.

@BalusC BalusC closed this as completed Jul 30, 2023
@BalusC BalusC modified the milestones: 4.1, 4.0 Jul 30, 2023
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Sep 7, 2023
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Sep 7, 2023
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Sep 7, 2023
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Sep 7, 2023
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Sep 8, 2023
Fixed regression error which caused spec1567IT to fail; the composite
instance is needed further down in ComponentNotFoundException block
arjantijms added a commit to eclipse-ee4j/mojarra that referenced this issue Sep 8, 2023
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