So this is really bizarre.

The log is saved when abuse filter is tripped, and then updated after the content is saved. But the way it knows which log id is based on a static variable that saves the log id between the two hooks. Somehow the log id for the first edit (which was not saved) was used for the second edit (which was saved).

@dbarratt — In which sequence should we resolve T173977: AbuseFilter tripping more times than it should, and for the wrong edit, and not tagging and this ticket?

In T177112#3658387, @TBolliger wrote:

@dbarratt — In which sequence should we resolve T173977: AbuseFilter tripping more times than it should, and for the wrong edit, and not tagging and this ticket?

Whichever. I don't think they are dependent on each other.

This is interesting. The diff happened a day before the entires in the log (I had it backwards in my previous comment).

It looks like what is happening is that PageContentSaveComplete is passing the old revision (i.e. the last one that was successful, not the current one which was not successful).

It looks like the mystery was also found in T128838: Investigate drop in revision_create events / apparent high rate of null edits:

In T128838#2096354, @GWicke wrote:

We looked into this a bit further last Friday. The documentation for PageContentSaveComplete mentions that the $revision parameter *can* be null if the content was not changed. There is an early return in the WikimediaEvents extension that is intended to trigger in that case. However, looking at the MediaWiki side, it looks like [the $revision parameter actually ends up being defined even if the content has not changed](https://github.com/wikimedia/mediawiki/blob/996b7350f3355f24abb55394ced108c0c9b8ef3a/includes/page/WikiPage.php#L1869).

And it is also documented:

$revision: Revision object of the saved content. If the save did not result in the creation of a new revision (e.g. the submission was equal to the latest revision), this parameter may be null (null edits, or "no-op"). However, there are reports (see phab:T128838) that it's instead set to that latest revision.

Also, this is related to T34948: {{REVISIONID}} and related magic words should not be blank after doing a null edit.

It looks like the best way to fix this is to use NewRevisionFromEditComplete instead of PageContentSaveComplete

Well... I thought I could reproduce this by making a null edit, but that doesn't seem to be the case.

IIRC this could have been caused by null edits under certain conditions. We observed something similar in T240115, and also in T168736, which took me quite a while to investigate. I should note that it's still possible for null edits to go through AbuseFilter under rare edge cases [1], but that's because we don't perform a full PST. This is related to T198651, and the patch for that tasks tried to implement the PST thing. That's currently stuck because I think it may have performance drawbacks.

For now, I'm adding a TestMe so this task can be investigated during the AbuseFilter overhaul and then closed or updated, depending on the current situation.

[1] - For instance, adding {{subst:void|...}}, or replacing a signature you just left on the page with ~~~~, or any other text which doesn't change the page content, but that you must first run through the parser to know that.