struts-community-plugins / struts2-jquery

Struts2 jQuery Plugin
Apache License 2.0
83 stars 49 forks source link

issue #308 make id field dynamic again #309

Closed sdutry closed 1 year ago

sdutry commented 1 year ago

fixes #308

sdutry commented 1 year ago

looking into changing it back and using escapedId as you suggested

sdutry commented 1 year ago

this will mean a change in all templates where id is used

sdutry commented 1 year ago

@lukaszlenart am i correct in concluding that i'll have to add the escaped_id parameter myself using the escape function and then in the ftl templates use the escapedId parameter?

lukaszlenart commented 1 year ago

@lukaszlenart am i correct in concluding that i'll have to add the escaped_id parameter myself using the escape function and then in the ftl templates use the escapedId parameter?

I think you can just omit the else statement like this:

protected void addGeneratedIdParam(String prefix) {
    if (StringUtils.isBlank(this.id)) {
        // resolves Math.abs(Integer.MIN_VALUE) issue reported by FindBugs
        // http://findbugs.sourceforge.net/bugDescriptions.html#RV_ABSOLUTE_VALUE_OF_RANDOM_INT
        int nextInt = RANDOM.nextInt();
        nextInt = nextInt == Integer.MIN_VALUE ? Integer.MAX_VALUE : Math.abs(nextInt);
        this.id = prefix + String.valueOf(nextInt);
        addParameter(PARAM_ID, this.id);
        addParameter("escapedId", this.id);
    }
}

and then use parameters.escapedId in the template.

Yet I think such change is good for a minor release not just a bug fix :)

lukaszlenart commented 1 year ago

I prepared my change also addressing some deprecation warnings #310