Click fork: half the upstream imports were never repointed, so type checks in ContainerUtils and Form can never match #1110

Rewritten after a full sweep of the fork. The original report named two sites in Form.java; the root cause turns out to be systematic, so the scope below is the whole pattern. The javax-bound half is split out into a separate issue — see the end.

org.openidentityplatform.openam.click is a copy of Apache Click 2.3.0 taken in-tree so the framework could be moved from javax.servlet to jakarta.servlet. During that copy, imports were repointed at the fork class by class, and only about half were done.

Measured over the 57 files of the fork:

util/ContainerUtils.java:38-49 shows the pattern in a single import block: Control, Container, Field and Form are the fork's, while Button, Label, FieldSet, LogService, ClickUtils and HtmlStringBuffer are still upstream.

Why the mixed imports break behaviour

The fork's Button, Label and ActionLink extend the fork's Field → AbstractControl → Control. They have no relationship to the upstream classes of the same name. Container.getControls() returns List<Control> of the fork Control, and add(Control) accepts only that type, so an upstream control can never be inside a fork container.

Every instanceof against an upstream type over that collection is therefore a constant false, and every guard built on one silently does nothing.

Filters that never fire

The same logic in Form.addStatefulFields:3051 — control instanceof Label || control instanceof Button — is correct, because Form.java does not import the upstream Label/Button and they resolve inside the fork's own package. Two files carrying the same code behave differently; that is the signature of the half-finished repoint.

Branches that can never be taken

• Form:3061 — control instanceof org.apache.click.control.Container, while the cast on the next line is to the fork Container. The recursion is dead, so addStatefulFields() does not keep the promise in its javadoc to "recursively include any Fields contained in child containers", and getState()/setState() (Form:1887, :1915) skip every field nested in a child container. Had the condition ever matched, the cast would throw ClassCastException.
• Form:2526-2527 (renderControls()), Field:315, :747 (isDisabled(), isReadonly()), ContainerUtils addInputFields/addHiddenFields — all test instanceof FieldSet against the upstream class. The fork ships no FieldSet at all, so the <td> styling and the parent-walk never run.

Reachability

None of this is a live defect today. No Click Form is constructed anywhere in the product — the only HiddenField instances are the framework's own NonProcessedHiddenField inside Form.java itself — and the pages the fork's servlet serves (the configurator and upgrader) create no controls. See #1109 for the full argument. The configurator works because it exercises the one path that was repointed completely.

Suggested scope

1. Repoint the 16 upstream imports that already have a fork class of the same name: ActionLink, Button, Label, Decorator, AbstractContainer, ClickUtils, HtmlStringBuffer, ConfigService, LogService, FileUploadService, ClickResourceService, VelocityTemplateService, DefaultMessagesMapService, CssStyle, JsImport, JsScript. This is mechanical and fixes every row of the table above.
2. Decide what to do about FieldSet: add a fork class or delete the branches and the import.
3. Fix Form:3061 to name the fork Container, matching the cast below it.
4. Drop the 20 javadoc-only imports that point at classes the fork does not ship (Submit, Reset, Checkbox, Panel, …), or repoint the ones that do have an equivalent.
5. Regression cover: getInputFields() and getButtons() over a container holding a fork Button and Label give a directly assertable result. There is currently no test anywhere under openam-core/src/test/java/org/openidentityplatform/openam/click/ except the one added by Remove the Java serialization round trip from the bundled Click HiddenField #1109.
6. Closing step: with the imports repointed, drop org.apache.click:click-nodeps:2.3.0 from openam-core. It cannot be removed before then — the fork imports 32 classes from that jar and does not compile without it.

Out of scope here

The other 16 upstream classes have no fork equivalent, and three of them (Format, MessagesMap, and the calls to org.apache.click.util.ClickUtils.getLogService()) reach upstream code that is bound to javax.servlet, which is not on the classpath. That is a different failure mode — NoClassDefFoundError rather than a branch quietly not taken — and needs its own fix. Tracked separately.