Skip to content

UI-Component: fix null value input-handling for group in group - #11869

Closed
iszmais wants to merge 1 commit into
ILIAS-eLearning:release_11from
iszmais:bugfix/nullable-group
Closed

UI-Component: fix null value input-handling for group in group#11869
iszmais wants to merge 1 commit into
ILIAS-eLearning:release_11from
iszmais:bugfix/nullable-group

Conversation

@iszmais

@iszmais iszmais commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@thibsy thibsy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iszmais I deliberately shifted the null check outside of isClientSideValueOk(). I know that for groups this is somewhat special, since are not sharing the same symbol, but could you find a solution that leaves this function (or the equivalent with _*) as is? Otherwise this would (to my knowledge) be the only case where null is checked inside this method. Kind regards, @thibsy (as UI coordinator)

@iszmais

iszmais commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@thibsy Thank you for your feedback.

After your feedback I tested this for other inputs and came to the conclusion that this error occurs for every input in groups (not just other groups). Therefore, it's correct that there is currently no other value check that considers null, but its seems that this is indeed a bug, means actually should every check include this.

The origin of the problem is that the group does not call the same validation method as the form itself, therefore, the validation inside the group diverges from the validation on the form layer.

That shouldn't be the case.
So either the group also needs to call a combine checkArg for validation (e.g. $this->checkArg("value", null === $value || $this->isClientSideValueOk($value)....
Or every Input should explicitly handle the null value.

For me, the second solution seems better conceptionally since there are also inputs that dont allow null as a value (see https://mantis.ilias.de/view.php?id=44169) and due to that every input should handle that individually to align the behavous inside and outside of a group.

Greetings,
@iszmais

@thibsy

thibsy commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Hi @iszmais,

You and I actually solved the Mantis issue that you mentioned with #11698. So if there is still an input left that does not handle null this way, I would consider this a bug.

But I am a little confused now, because you said

So either the group also needs to call a combine checkArg for validation (e.g. $this->checkArg("value", null === $value || $this->isClientSideValueOk($value)....
Or every Input should explicitly handle the null value.

Which is exactly what we should have implemented in the PR above. Did you check this behaviour with the latest release_11?

Thx for your clarification.

Kind regards,
@thibsy (as UI coordinator)

@iszmais

iszmais commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@thibsy Thanks for your response!

This may be the case. I guess I lost overview over the situation.
Nevertheless, this is still an issue since the null value is not checked on "isClientSideValueOk" but on "withValue" which is a conceptional flaw IMO.
This flaw is exposed withing the group client value check since this does, of course not call a function that sets a value but on that validates it (which it doesn't).

However, I’m totally open that this might not be the desired solution. But something similar has to.

Greetings,
@iszmais

@thibsy

thibsy commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

This is superseded by #11953

@thibsy thibsy closed this Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix kitchen sink php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants