Skip to content

Move isValidEvent into Form and FieldsetElement and add ON_ASSEMBLED support - #207

Merged
lippserd merged 3 commits into
mainfrom
form-elements-on-assembled-event
Jul 30, 2026
Merged

lippserd merged 3 commits into
mainfrom
form-elements-on-assembled-event

Conversation

@jrauh01

@jrauh01 jrauh01 commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

Form and FieldsetElement previously rejected ON_ASSEMBLED as an invalid event, causing on(HtmlDocument::ON_ASSEMBLED, ...) to throw an InvalidArgumentException on both classes. This moves isValidEvent out of the FormElements trait and into each concrete class so the allowed set can be scoped correctly per class.

Form accepts the full set of form lifecycle events plus ON_ASSEMBLED. FieldsetElement is restricted to ON_ELEMENT_REGISTERED and ON_ASSEMBLED, the only two events it actually emits. Previously the shared trait list caused FieldsetElement to accept events it never dispatches: ON_SUBMIT, ON_SENT, ON_ERROR, ON_REQUEST, and ON_VALIDATE. Registering listeners for those events would silently do nothing. They now correctly throw. This is a breaking change for any code that registered such listeners on a FieldsetElement.

Any future class that uses the FormElements trait without providing its own isValidEvent override will fall through to Events::isValidEvent, which accepts all event names unconditionally. That class must provide its own override to enforce a meaningful restriction.

@jrauh01 jrauh01 self-assigned this Jul 30, 2026
@cla-bot cla-bot Bot added the cla/signed label Jul 30, 2026
jrauh01 added 3 commits July 30, 2026 10:46
…Element`

The trait's shared list included form lifecycle events (`ON_SUBMIT`,
`ON_SENT`, `ON_ERROR`, `ON_REQUEST`, `ON_VALIDATE`) that `FieldsetElement`
never emits. Keeping a single list in the trait forced `FieldsetElement`
to accept events it has no way of dispatching.

By moving the check into each class, the allowed set can be scoped
correctly per class. `FieldsetElement` is restricted to `ON_ELEMENT_REGISTERED`,
the only form-elements event it actually emits.
`Form::ON_ELEMENT_REGISTERED` coupled the trait to `Form` specifically,
even though the trait is also used by `FieldsetElement`. Using `static::`
resolves the constant through the using class, consistent with how
`HtmlDocument` emits `ON_ASSEMBLED`.
@jrauh01
jrauh01 force-pushed the form-elements-on-assembled-event branch from 3b02120 to 1c7824e Compare July 30, 2026 08:46
@jrauh01
jrauh01 requested a review from lippserd July 30, 2026 08:55

@lippserd lippserd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 1c7824e finally makes the Form import unused, right?

@jrauh01

jrauh01 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

No it doesn't, there's still a @param annotation. But since it's the only one now we could consider replacing it by fully qualified class name.

@lippserd

Copy link
Copy Markdown
Member

No it doesn't, there's still a @param annotation. But since it's the only one now we could consider replacing it by fully qualified class name.

It's fine. Let's keep it as is.

@lippserd
lippserd merged commit a411d59 into main Jul 30, 2026
13 checks passed
@lippserd
lippserd deleted the form-elements-on-assembled-event branch July 30, 2026 15:01
@lippserd lippserd added this to the v0.12.0 milestone Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants