Page MenuHomePhabricator

Rollback confirmation "confirmable" breaks when list on RecentChanges / RelatedChanges is updated
Closed, ResolvedPublic13 Estimated Story Points

Description

When accessing RecentChanges as a user with Rollback rights, the rollback links will work properly on page laod. However, when updating the list of items shown on the RecentChanges page via the filters on the right (for example, increasing the number of "Results to show" or changing the time period that is shown) or by letting new elements appear through the live update feature, new items will not have the rollback confirmation JavaScript event attached to them, meaning users will be redirected when clicking on a rollback link rather than being asked to confirm their action on the page.

Affected users are still prompted to confirm the rollback as the non-JS rollback confirmation will take over, so this is not a critical issue and users can not accidentally rollback anything, it's mainly a user-experience problem.

The issue itself is kind of related to how the "Confirmables" work that we are using in the code to implement the user confirmation:

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/488048/33/resources/src/mediawiki.rollback.confirmation.js

As far as I can see, we can either fix this by making changes to the Confirmable script as the issue is strongly linked to it (since we are simply re-using it, much like the Thanks extension). Alternatively, we can also consider this a minor issue and postpone this until a better, more permanent solution is found for the user interface. However, I assume the latter will take too much time, so it may be best to first fix the current approach before we move forward.

A solution may be to implement delegated event handlers into the Confirmable script, but we may need to double-check this.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 14 2019, 8:20 PM
kai.nissen set the point value for this task to 13.Mar 18 2019, 11:21 AM
Tim_WMDE moved this task from Todo to Doing on the WMDE-FUN-Sprint-2019-03-18 board.
Tim_WMDE renamed this task from Rollback confirmation "confirmable" breaks when post list on RecentChanges is updated to Rollback confirmation "confirmable" breaks when list on RecentChanges / RelatedChanges is updated.Mar 18 2019, 2:50 PM

Change 497334 had a related patch set uploaded (by Tim Eulitz; owner: Tim Eulitz):
[mediawiki/core@master] Add optional jQuery event delegates in Confirmable

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

@matmarex I am especially interested in your feedback here, would be great if you could have a look at this if you find the time.

Tim_WMDE removed Tim_WMDE as the assignee of this task.Mar 20 2019, 8:58 AM
Tim_WMDE moved this task from Doing to Review on the WMDE-FUN-Sprint-2019-03-18 board.
Tim_WMDE removed a subscriber: Tim_WMDE.
Tim_WMDE added a subscriber: Tim_WMDE.

@Tim_WMDE Using delegated event handlers looks like a good solution. I would even consider changing the script to always use them, rather than as an option (but this would probably require a change in the Thanks extension, so it's reasonable to leave it alone).

@matmarex Thanks for your feedback. I was thinking the same thing but I saw that it's being used by three extensions as of right now. I purposefully kept it optional for the time being since I am not too familiar with how to introduce breaking changes / deprecations into the MediaWiki world:

https://codesearch.wmflabs.org/search/?q=confirmable%5C(&i=nope&files=&repos=

Change 497334 merged by jenkins-bot:
[mediawiki/core@master] Add optional jQuery event delegates in Confirmable

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

Change 498096 had a related patch set uploaded (by Tim Eulitz; owner: Tim Eulitz):
[mediawiki/core@master] Use delegated events for rollback confirmable

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

Change 498096 merged by jenkins-bot:
[mediawiki/core@master] Use delegated events for rollback confirmable

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

Change 498346 had a related patch set uploaded (by Gabriel Birke; owner: Gabriel Birke):
[mediawiki/core@master] Remove i18n parameters

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

Change 498346 merged by jenkins-bot:
[mediawiki/core@master] Remove i18n parameters

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

kai.nissen closed this task as Resolved.Apr 15 2019, 11:03 AM