Page MenuHomePhabricator

Rollback gets both "mw-rollback" and "mw-undo" tags
Closed, ResolvedPublic

Description

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.

Event Timeline

Aklapper renamed this task from Rollback gets two similar tags to Rollback gets both "mw-rollback" and "mw-undo" tags.Mar 24 2018, 10:10 AM

Change 422075 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/core@master] Revert "Pass revision being reverted to edit code"

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

Change 422078 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/extensions/Echo@master] Revert my fix for summary pings in reverts

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

Change 422079 had a related patch set uploaded (by MaxSem; owner: MaxSem):
[mediawiki/extensions/Echo@master] Detect reverts differently

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

Change 422078 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Revert my fix for summary pings in reverts

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

Change 422075 merged by jenkins-bot:
[mediawiki/core@master] Revert "Pass revision being reverted to edit code"

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

Change 422079 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Detect reverts differently

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

MaxSem moved this task from Ready to Q1 2018-19 on the Community-Tech-Sprint board.

Hello. The bug is back. See for example here. Thank you.

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.

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.

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.

"Applied manually by users and bots"

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

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

Change 443193 merged by jenkins-bot:
[mediawiki/core@master] WikiPage: Do not set "undid revision ID" for rollbacks

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

@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.

matmarex claimed this task.

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

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

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

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

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)

The fix is now deployed to Wikimedia wikis.