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

ajax b:commandButton loses AJAX if onclick is used #750

Closed
ggam opened this issue May 3, 2017 · 21 comments · Fixed by #1183
Closed

ajax b:commandButton loses AJAX if onclick is used #750

ggam opened this issue May 3, 2017 · 21 comments · Fixed by #1183
Assignees

Comments

@ggam
Copy link
Collaborator

ggam commented May 3, 2017

This xhtml:
<b:commandButton action="#{index.action}" value="Action" ajax="true" />

Generates the following (as expected):
<button type="submit" id="j_idt5:j_idt6" name="j_idt5:j_idt6" class="btn btn-default" onclick="BsF.ajax.callAjax(this, event,'j_idt5','j_idt5',null,null,null,null);;;return false;">Action</button>

But when setting the onclick attribute...
<b:commandButton onclick="alert(1)" action="#{index.action}" value="Action" ajax="true" />
We get the following:
<button type="submit" id="j_idt5:j_idt6" name="j_idt5:j_idt6" class="btn btn-default" onclick="alert(1);">Action</button>

The onclick is overriden and the AJAX call is lost. The workaround is to set the ajax call by hand:
<b:commandButton onclick="alert(1);ajax:index.action()" value="Action" ajax="true" />

This also affects other AJAX components, like inputText: <b:inputText value="hola" ajax="true" onchange="alert(1);" />

@stephanrauh
Copy link
Collaborator

I thought this is the expected behavior, but after checking what b:commandButton and prime:commandButton do, I agree with you that this is a bug.

@ggam
Copy link
Collaborator Author

ggam commented May 3, 2017

When I first saw it I couldn't classify it as a bug. But I didn't see it natural. If I set ajax to true, I expect the form to be submitted via ajax, independently of any other events.

@stephanrauh
Copy link
Collaborator

I frequently to something like onclick="if (condition) return false;". This snippet prevents the AJAX call conditionally. So it's a bit surprising I didn't chain the original AJAX call to the user-define JavaScript snippet.

@ggam
Copy link
Collaborator Author

ggam commented May 22, 2017

This is causing the following issues:

  • oncomplete doesn't work, as it just registers a callback for the action AJAX call, which is not rendered
  • JSF navigation only works for form actions. If you want a redirection as a result of the request, (calling a method returning a String), navigation won't be triggered for the onclick calls.

The workaround is manually making the AJAX request on the onclick using BootsFaces APIs, like if it was rendered from the action.

@stephanrauh
Copy link
Collaborator

IMHO we should chain the JavaScript and the AJAX call, just like Mojarra, MyFaces and PrimeFaces do it. We just haven't started to do so :).

@7homasSutter
Copy link

+1

1 similar comment
@t-oster
Copy link
Contributor

t-oster commented Nov 28, 2018

+1

@t-oster
Copy link
Contributor

t-oster commented Nov 28, 2018

I also stumbled on this bug and it was not clear to me. I first used <b:commandButton with some action and ajax set to true. Then I wanted to have an (client side) onclick action, but suddenly the button did not use ajax anymore. So at least if the onclick attribute does not use any serverside ajax calls, this is clearly a bug.

@t-oster
Copy link
Contributor

t-oster commented Nov 28, 2018

For clarification:

<!-- uses ajax as expected -->
<b:commandButton action="#{myBean.doSomething}" ajax="true" />
<!-- Does NOT use ajax, but should -->
<b:commandButton action="#{myBean.doSomething}" ajax="true" onclick="console.log('wtf')"/>
<!-- Workaround until this is fixed -->
<b:commandButton  onclick="console.log('wtf');ajax:myBean.doSomething()"/>

@vsvetoslavov
Copy link
Contributor

vsvetoslavov commented Apr 11, 2019

I see one thing here worth checking in @t-oster 's example above

<!-- Does NOT use ajax, but should -->
<b:commandButton action="#{myBean.doSomething}" ajax="true" onclick="console.log('wtf')"/>

It is not clear whether console.log('wtf'); should be called before or after the ajax event - therefore the workaround seems easier to understand.

@stephanrauh
Copy link
Collaborator

I agree that the workaround is easier to understand. That's why I invented ajax:some.method() in the first place. :) But even this workaround might be confusing. You have to keep in mind that ajax:what.ever() starts the call of what.ever() without waiting for the result. Every JavaScript programmer should know that, but there's no reason to assume every Java programmer is familiar with this pattern.

@t-oster
Copy link
Contributor

t-oster commented Apr 13, 2019

When using ajax in the onclick listener, I agree it is not clear, however if just using plain JS I would have assumed that it behaves like the usual on click handler,meaning the JS is evaluated first and after that the usual action is performed (as it already works with ajax=false)

@t-oster
Copy link
Contributor

t-oster commented Oct 22, 2019

What is the state of this issue? Is there any reason not to chain the user submittet 'onclick' with the javascript from the action? Maybe even only if the onclick evaluates to true. The current workaround is not very beautiful, does not work with autocomplete in most IDEs. I do not see any disadvantage in chaining.

@bytecode-seller
Copy link

I think this project is abandoned. The real workaround is this:

onclick="if(!confirm(‘Do you really want to delete this item?')) return false; ajax:catalogSatEstController.delete();javascript:return false;”

Both return false commands prevent page refresh and/or unwanted post while letting do required bean call.

@t-oster
Copy link
Contributor

t-oster commented Dec 29, 2021

@TheCoder4eu will you accept a pull request, which changes that behavior? If I find the time, I am willing to contribute this, but I want to be sure it will be merged.

@TheCoder4eu
Copy link
Owner

Hi @t-oster ,
sorry for the late answer, yes your contribution is very appreciated, so please go ahead with the pull request and we will merge it after a quick review!

@TheCoder4eu
Copy link
Owner

@t-oster ,
I forgot to add that we'd like to release v1.6.0 soon :-)

@t-oster
Copy link
Contributor

t-oster commented Jan 10, 2022

The underlying issue is in AjaxRenderer.generateBootsFacesAJAXAndJavaScript

public static void generateBootsFacesAJAXAndJavaScript(FacesContext context, ClientBehaviorHolder component,
method:
It first calls generateAJAXCallForASingleEvent here
generatedAJAXCall |= generateAJAXCallForASingleEvent(context, component, rw, null, specialEvent,
, which calls the ResponseRenderer.renderAttribute method with the original onclick attribute.
Afterwards it generates the correct javascript and again calls the ResponseRenderer.renderAttribute method here
rw.writeAttribute("on" + defaultEvent, script, null);
, but that seems to have no effect (probably the attribute is only added on the first call).

I could inject a Decorator for the ResponseRenderer class, which caches all renderAttribute calls and overwrites previous versions with later ones, but this does not make the code very readable.

On the other hand I cannot see an easy way to swap the order of the calls, because the first call determines if the onclick handler already includes ajax.

@t-oster
Copy link
Contributor

t-oster commented Jan 10, 2022

I have a better idea, will update the PR tomorrow: if A
ajax = true, and no ajax: is used in the onclick handler, just skip the first writeAtttibute call. Are there any unit tests?

@qbtcoder
Copy link

Hi @t-oster ,
thank you for your PR!
We'll wait for the update, unfortunately there are no unit tests.

@t-oster
Copy link
Contributor

t-oster commented Jan 12, 2022

The second version is probably easier to read.

TheCoder4eu pushed a commit that referenced this issue Jan 23, 2022
geopossachs pushed a commit to geopossachs/BootsFaces-OSP that referenced this issue Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants