Indeed. PageChangeEventIngress only emits visibilty change events if the current revision is changed. Filtering out historical revision visibilty changes should not be an exception!

But, I see that PageChangeEventIngress does

			// Read from primary since due to replication lag the updated field visibility
			// might not yet be available on a replica, and we are at risk of leaking
			// just suppressed data.
			$revisionRecord = $this->revisionStore->getRevisionByPageId(
				$event->getPageId(),
				0, // <- this is $rev_id
				IDBAccessObject::READ_LATEST
			);

Passing 0 as $rev_id to getRevisionByPageId causes getRevisionByPageId to always return the latest revision. It looks like this was done because the PageHistoryVisibilityChangedEvent does not provide public access to $currentRevisionId.

@daniel, should PageHistoryVisibilityChangedEvent have a getRevisionId (or getRevisionRecord?) method?

Oh, hm. Maybe not. getAffectedRevisionIDs is more appropriate. I think I see. Thanks.

Actually, I think having the 'current revision' as PageHistoryVisibilityChangedEvent sees it would be helpful. In PageChangeEventIngress, we get the latest current revision from the master. I suppose it is possible for this revision to be different than the one that was 'current' when the PageHistoryVisibilityChangedEvent was constructed. We do want to emit an event in all cases where the 'current' revision visibility changes.

Since we have to fetch a RevisionRecord, it would be best to fetch the one that PageHistoryVisibilityChangedEvent has as the current revision, rather than the latest one in master database.

I could work around this by first checking $event->wasCurrentRevisionAffected(), and then using the max (event->getAffectedRevisionIDs()) ? Assuming that if the current revision affected, the max affected rev_id would be the current revision?

Change #1191452 had a related patch set uploaded (by Ottomata; author: Ottomata):

[mediawiki/extensions/EventBus@master] PageChange - Fix logic in handlePageHistoryVisibilityChangedEvent

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

Change #1191484 had a related patch set uploaded (by Ottomata; author: Ottomata):

[mediawiki/core@master] PageHistoryVisibilityChangedEvent - add getCurrentRevisionId method

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

I submitted a patch to MW core to add getCurrentRevisionId to PageHistoryVisibiltyChangedEvent.

Change #1191484 merged by jenkins-bot:

[mediawiki/core@master] PageHistoryVisibilityChangedEvent - add getCurrentRevisionId method

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

Change #1191452 merged by jenkins-bot:

[mediawiki/extensions/EventBus@master] PageChange - Fix logic in handlePageHistoryVisibilityChangedEvent

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