Am 30.05.2014 15:38, schrieb Brad Jorsch (Anomie):
I think you need to look again into how FlaggedRevs
uses it, without the
preconceptions you're bringing in from the way you first interpreted the
name of the variable. The current behavior makes perfect sense for that
specific use case. Neither of your proposals would work for FlaggedRevs.
As far as I understand the rather complex FlaggedRevs.hooks.php code, it assumes
a) if $newRev === $baseRevId, it's a null edit. As far as I can see, this does
not work, since $baseRevId will be null for a null edit (and all other regular
b) if $newRev !== $baseRevId but the new rev's hash is the same as the base
rev's hash, it's a rollback. This works with the current implementation of
commitRollback(), but does not for manual reverts or trivial undos.
So, FlaggedRevs assumes that EditPage resp WikiPage set $baseRevId to the edits
logical parent (basically, the revision the user loaded when starting to edit).
That's what I described as option (3) in my earlier mail, except for the
rollback case; It would be fined with me to use the target rev as the base for
rollbacks, as is currently done.
FlaggedRevs.hooks.php also injects a baseRevId form field and uses it in some
cases, adding to the confusion.
In order to handle manual reverts and null edits consistently, EditPage should
probably have a base revision as a form field, and pass it on to doEditContent.
As far as I can tell, this would work with the current code in FlaggedRevs.
As for the EditPage code path, note that it has
already done edit conflict
resolution so "base revision = current revision of the page". Which is
probably the intended meaning of false.
Right. If that's the case though, WikiPage::doEditContent should probably set
$baseRevId = $oldid, before passing it to the hooks.
Without changing core, it seems that there is no way to implement a late/strict
conflict check based on the base rev id. That would need an additional "anchor
revision" for checking.
The easiest solution for the current situation is to simply drop the strict
conflict check in Wikibase and accept a race condition that may cause a revision
to be silently overwritten, as is currently the case in core.