Page MenuHomePhabricator

Add rollback confirmation code to existing module instead of registering a new one
Closed, ResolvedPublic2 Estimated Story Points

Description

As pointed out by @Krinkle, since the code for the rollback confirmation feature is trivial in size and is not worth slowing down all page views for for all users by registering an extra module, we want to merge the module with an existing one to increase the server side performance and move the relevant checks into the client-side code. The relevant user preferences can then be checked via mw.user.options.

Event Timeline

Hey @Krinkle, I have looked into the suggestion you have made here: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/488048/37#message-b5d7f436a36b59c659e5d59a902e0f9d4e89c747

I don't really see a way for this JavaScript snippet to be placed in all the right places unless we really place it in a top-level JavaScript file where it's executed on every page load which does not seem right either. Initially I thought that the mediawiki.page.rollback module would be a good fit, but when looking into that module, I realized that it's actually unused, as the patch that introduced it here was partially rolled back later while the module and its mediawiki.page.rollback.js were left in the code without being used:

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/242050

In the patch above, the module was previously also loaded in the \Linker::generateRollback()` just like we do now, it simply makes the most sense for this use case, as far as I can see. Let me know if you have any remarks / suggestions about this, maybe I missed something.

Change 499226 had a related patch set uploaded (by Tim Eulitz; owner: Tim Eulitz):
[mediawiki/core@master] Remove unused rollback module

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

Tim_WMDE set the point value for this task to 2.Mar 26 2019, 3:30 PM
Tim_WMDE moved this task from Doing to Review on the WMDE-FUN-Sprint-2019-03-18 board.
Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.

Yep. That's fine, and addresses the underlying concern just as well. Nice find :)

Change 499226 merged by jenkins-bot:
[mediawiki/core@master] Remove unused rollback module

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

Krinkle claimed this task.
Krinkle removed a project: Patch-For-Review.