Effect#statementArgs() keeps adding callback parameter

[GoogleCodeExporter]

The Effect#statementArgs() method keeps adding the rendered callback parameter 
to its internal List of parameters. Because of this behavior, the method can't 
be used more then once on the same Effect instance.

In some situations using a stateful page (i.e. with a simple Link with onClick 
handler), the Effect instance is restored from session and Wicket/WiQuery will 
eventually call statementArgs() again. This results in the callback parameter 
appearing twice in the JavaScript call.

I'd suggest storing the original parameters (from the constructor, effect speed 
and callback) separately to avoid this problem.

(WiQuery 1.2.3, Wicket 1.4.16 on GlassFish 2.1.1)

Original issue reported on code.google.com by norc...@gmail.com on 22 Mar 2011 at 11:51

[GoogleCodeExporter]

Hi,

I think I will change the statementArgs to prevent the populating of the list 
with the effect callback:

/**
     * {@inheritDoc}
     * @see org.odlabs.wiquery.core.javascript.ChainableStatement#statementArgs()
     */
    public CharSequence[] statementArgs() {
        int size = this.parameters.size();
        boolean effectIsSet = this.effectCallback() != null;

        if(effectIsSet) {
            size++;
        }

        CharSequence[] args = new CharSequence[this.parameters.size()];
        int count = 0;

        for (CharSequence charSequence : this.parameters) {
            args[count] = charSequence;
            count++;
        }

        if(effectIsSet) {
            args[count + 1] = this.effectCallback().render();
        }

        return args;
    }

What do you think about this ?

Thanks

Regards

Julien Roche

Original comment by roche....@gmail.com on 26 Mar 2011 at 8:50

• Changed state: Accepted
• Added labels: Milestone-Release1.2.3

[GoogleCodeExporter]

Oups, I mean:

/**
     * {@inheritDoc}
     * @see org.odlabs.wiquery.core.javascript.ChainableStatement#statementArgs()
     */
    public CharSequence[] statementArgs() {
        int size = this.parameters.size();
        boolean effectIsSet = this.effectCallback() != null;

        if(effectIsSet) {
            size++;
        }

        CharSequence[] args = new CharSequence[this.parameters.size()];
        int count = 0;

        for (CharSequence charSequence : this.parameters) {
            args[count] = charSequence;
            count++;
        }

        if(effectIsSet) {
            args[count] = this.effectCallback().render();
        }

        return args;
    }

Original comment by roche....@gmail.com on 26 Mar 2011 at 8:59

[GoogleCodeExporter]

And of course use the size :) That should do it!
Thanks.

        int size = this.parameters.size();
        boolean effectIsSet = (this.effectCallback() != null);

        if (effectIsSet) {
            size++;
        }

        CharSequence[] args = new CharSequence[size];
        int count = 0;

        for (CharSequence charSequence : this.parameters) {
            args[count] = charSequence;
            count++;
        }

        if (effectIsSet) {
            args[count] = this.effectCallback().render();
        }

        return args;

Original comment by norc...@gmail.com on 29 Mar 2011 at 11:56

[GoogleCodeExporter]

Roger that

I will push the patch tonight.

Best regards

Julien Roche

Original comment by roche....@gmail.com on 29 Mar 2011 at 11:59

[GoogleCodeExporter]

Original comment by roche....@gmail.com on 29 Mar 2011 at 11:59

• Added labels: Milestone-Release1.2.4
• Removed labels: Milestone-Release1.2.3

[GoogleCodeExporter]

Fix in commit r737

Original comment by roche....@gmail.com on 31 Mar 2011 at 8:41

[GoogleCodeExporter]

Original comment by roche....@gmail.com on 31 Mar 2011 at 8:41

• Changed state: Fixed

[GoogleCodeExporter]

Sorry but the commit was a bit dodgy, it crashed the tests in 1.5. I altered 
the code so it is safer and easier to read.

Original comment by hielke.hoeve on 6 Apr 2011 at 2:23