Hello. Starting with this week deployment, four hours ago, users complain that rollbacks get both mw-rollback and mw-undo tags.
See for example https://he.wikipedia.org/w/index.php?diff=22669463&uselang=en.
Description
Details
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Open | None | T125653 Create new types of notifications | |||
Resolved | None | T32750 [Epic] Ping/notify user when username used in an edit summary | |||
Resolved | matmarex | T190374 Rollback gets both "mw-rollback" and "mw-undo" tags |
Event Timeline
This is certainly a regression from rMWb7737ebed1e8: Pass revision being reverted to edit code.
Change 422075 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/core@master] Revert "Pass revision being reverted to edit code"
Change 422078 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/extensions/Echo@master] Revert my fix for summary pings in reverts
Change 422079 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/extensions/Echo@master] Detect reverts differently
Change 422078 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Revert my fix for summary pings in reverts
Change 422075 merged by jenkins-bot:
[mediawiki/core@master] Revert "Pass revision being reverted to edit code"
Change 422079 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Detect reverts differently
And one thing more. I do not know if it's the same bug or not. If it isn't, i'll open a new bug. When a patroller rollbacks an edit, it is not marked as patrolled. See for example here. Thank you.
I have opened a bug about this. It is something different. I am commenting from mobile, see my recent activity for details.
With rMWc7564daa80293f38cf04d54e58464e0dbb8b82a0, both WikiPage::doEditContent and WikiPage::commitRollback now do:
$updater->setUndidRevisionId( $undidRevId );
Then, this happens in PageUpdater:
if ( $this->undidRevId !== 0 && in_array( 'mw-undo', ChangeTags::getSoftwareTags() ) ) { $tags[] = 'mw-undo'; }
I would leave out the setter in commitRollback for now. Note that rollback will actually undo multiple revisions, not just a single one.
I would leave out the setter in commitRollback for now.
Since line 2944 of the old code did not set the $undidRevId parameter in the call to doEditContent(), that would restore the previous behavior.
Note that rollback will actually undo multiple revisions, not just a single one.
Undo can also do this, same problem.
I added this because omitting $undidRevId heree seemed like an oversight. Semantically, rollbacks are really a special kind of undo.
I still think having both flags on rollbacks makes sense, but I did not intent to change UI behavior here without prior discussion. So I agree that the call to setUndidRevisionId() should be removed for now. Perhaps a TODO or XXX comment should stay in place, with a reference to this ticket.
Does anyone know how the "rv" tag on the Finnish Wiki works. There are several types of reverts including one whose edit summary begins with "Hylättiin viimeisin ..." but is not tagged as either an undo (Kumottu) nor a rollback (Palautettu takaisinpäin). This would complicate matters further when I notice here that every rollback in article space now has three tags, while those in other namespaces lack the "rv" tag but has the other two.
It looks like Matěj has already proposed a fix, and Daniel has already approved it, so let me do the honors and push it to Gerrit…
Change 443193 had a related patch set uploaded (by Bartosz Dziewoński; owner: Matěj Suchánek):
[mediawiki/core@master] WikiPage: Do not set "undid revision ID" for rollbacks
Change 443193 merged by jenkins-bot:
[mediawiki/core@master] WikiPage: Do not set "undid revision ID" for rollbacks
@matmarex thank you for the quick merge. Would it be possible to push it to production branch (I think 1.32.0-wmf.10?)
I think this should wait for train, as this isn't urgent, it is almost just a visual problem.
The problem re-appeared. I'm moving it back to "To Triage" column on the User-notice board.
Looks like the fix is on master, but was not backported. I suppose it will go out with the regular train then. You can request it to be SWATed if you think it's urgent.
Change 443462 had a related patch set uploaded (by Bartosz Dziewoński; owner: Matěj Suchánek):
[mediawiki/core@wmf/1.32.0-wmf.10] WikiPage: Do not set "undid revision ID" for rollbacks
I'd rather backport it, it is a small patch for an annoying (if minor) issue, and there are no train deployments this week.
Change 443462 merged by jenkins-bot:
[mediawiki/core@wmf/1.32.0-wmf.10] WikiPage: Do not set "undid revision ID" for rollbacks
Mentioned in SAL (#wikimedia-operations) [2018-07-02T19:00:36Z] <niharika29@deploy1001> Synchronized php-1.32.0-wmf.10/includes/page/WikiPage.php: Mark rollbacking revision as patrolled T198449; WikiPage: Do not set undid revision ID for rollbacks T190374 (duration: 00m 50s)