The old code used WebRequest::getCheck and the new code uses ParamValidator which does not even invoke the type class if the value is null. This seems especially confusing for the PresenceBooleanDef (checkbox) type which is documented as

This boolean is considered true if the parameter is present in the request, regardless of value. The only way for it to be false is for the parameter to be omitted entirely.

but actually it will be null when omitted entirely.

Meh. For good measure I checked Wikimedia-deployed extensions and didn't see anything that would be affected by this. But it seems easy enough to fix.

In T244440#5854722, @Tgr wrote:

the new code uses ParamValidator which does not even invoke the type class if the value is null

It always invokes the TypeDef to fetch the value from the request, but doesn't call the validate method if the value is null.

This seems especially confusing for the PresenceBooleanDef (checkbox) type which is documented as

In the Action API, that's the type called "boolean" and is the only boolean type available.

All PresenceBooleanDef::validate() does is cast the value to boolean anyway, so it's not like there's anything really missing by not calling it.

In T244440#5856030, @Krinkle wrote:

@Krinkle added a parent task: T233867: 1.35.0-wmf.19 deployment blockers

This seems to have no effect on anything. Why do you claim it's a blocker?

Change 570662 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] ParamValidator: Default PresenceBooleanDef to false rather than implicitly null

https://gerrit.wikimedia.org/r/570662

Train Blocker = UBN! ?

Change 570662 merged by jenkins-bot:
[mediawiki/core@master] ParamValidator: Default PresenceBooleanDef to false rather than implicitly null

https://gerrit.wikimedia.org/r/570662

In T244440#5856312, @Anomie wrote:

This seems especially confusing for the PresenceBooleanDef (checkbox) type which is documented as

In the Action API, that's the type called "boolean" and is the only boolean type available.

How so? ApiBase passes the type unchanged to ParamValidator, which maps boolean to BooleanDef. So I don't think the patch changes anything.

In T244440#5857821, @Tgr wrote:

How so? ApiBase passes the type unchanged to ParamValidator, which maps boolean to BooleanDef. So I don't think the patch changes anything.

Look at includes/api/Validator/ApiParamValidator.php, in particular line 46.

Ah, thanks. I missed that ApiMain uses an ApiParamValidator, not a ParamValidator.

@AMooney, I understand that this ticket is resolved. Should it be in the Done column instead of the Waiting for Review one?