UserrightsPage (SpecialUserrights.php) is not safe to extend, but it is extended by:
- SpecialGlobalGroupMembership in MediaWiki-extensions-CentralAuth
- GlobalUserrights in MediaWiki-extensions-GlobalUserrights
Description
Description
Details
Details
Related Changes in Gerrit:
Customize query in gerrit
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Declined | None | T252814 Unused local "stewards" group (finding situations that local stewards groups are used) | |||
| Stalled | Zabe | T14518 Cross-wiki userrights should use groups from target wiki instead local wiki | |||
| Open | None | T263207 The `UserrightsPage` class is not safe to extend |
Event Timeline
Comment Actions
Discovered as a complication for T14518: Cross-wiki userrights should use groups from target wiki instead local wiki
Comment Actions
No, I recommend splitting out whatever functionality needs to be available to extensions to a separate class/trait/service
Comment Actions
Change 642859 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Revert "Inject services into SpecialUserrights"
Comment Actions
From the latest revert
the problem is that SpecialGlobalGroupMembership doesn't actually call parent::__construct() and so none of the fallbacks provided for in the constructor were used, resulting in
[X7xOV6wQBHcAAE5b7KoAAAAC] /wiki/Special:GlobalUserRights/DannyS712 Error from line 143 of /srv/mediawiki/php-master/includes/specials/SpecialUserrights.php: Call to a member function getCanonical() on null
Backtrace:
#0 /srv/mediawiki/php-master/includes/specialpage/SpecialPage.php(639): UserrightsPage->execute(string)
#1 /srv/mediawiki/php-master/includes/specialpage/SpecialPageFactory.php(1272): SpecialPage->run(string)
#2 /srv/mediawiki/php-master/includes/MediaWiki.php(310): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#3 /srv/mediawiki/php-master/includes/MediaWiki.php(945): MediaWiki->performRequest()
#4 /srv/mediawiki/php-master/includes/MediaWiki.php(548): MediaWiki->main()
#5 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#6 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#7 /srv/mediawiki/w/index.php(3): require(string)
#8 {main}Comment Actions
Change 642859 merged by jenkins-bot:
[mediawiki/core@master] Revert "Inject services into SpecialUserrights"
Comment Actions
Change 643299 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/CentralAuth@master] Prepare special page constructor for service injection in core
Comment Actions
Change 643302 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/GlobalUserrights@master] Prepare special page constructor for service injection in core
Comment Actions
Change 643299 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Prepare special page constructor for service injection in core
Comment Actions
Change 643155 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Reapply "Inject services into SpecialUserrights"
Comment Actions
Change 643302 merged by jenkins-bot:
[mediawiki/extensions/GlobalUserrights@master] Prepare special page constructor for service injection in core
Comment Actions
Change 643155 merged by jenkins-bot:
[mediawiki/core@master] Reapply "Inject services into SpecialUserrights"
Comment Actions
UserrightPage special in core uses cross-wiki User access via a UserrightsProxy, which is ill-documented and ill-supported concept. With introduction of ActorStore, we are going to refactor UserrightPage.
In order to even approach doing that we need to make make SpecialGlobalGroupMembership not extend the UserrightsPage. As a first step in this, we need to just make SpecialGlobalGroupMembership extend SpecialPage and copy all of the methods inherited from UserrightsPage directly into it. This will be an intermediate step in this refactoring, as we standardize on cross-wiki user access, we will refactor CentralAuth to use it as well.
Comment Actions
Change 738574 had a related patch set uploaded (by Zabe; author: Zabe):
[mediawiki/extensions/CentralAuth@master] Stop extending the UserrightsPage class
Comment Actions
Do we need to fix GlobalUserrights too, not being deployed or (apparently) actively maintained?
Comment Actions
Change 738574 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Stop extending the UserrightsPage class
Comment Actions
To answer my own question: I was going to do that anyway, then noticed that the extension has a compatibility policy of "master" and supports MW >= 1.35. I honestly don't want to copy 900 lines of code from master and make sure that each of them would still work on 1.35, adding BC hacks if not. I think it's not unfair to let the maintainers do whatever they think is best, be it bumping the minimum supported version or adding the BC hacks.
Comment Actions
It's very much appreciated! (I'm not sure what kind of active maintaining it needs or what, if anything, is broken; but it's very much used at ShoutWiki and hence should be compatible with the latest LTS release...or if not, then that certainly is a problem.)
Comment Actions
(Didn't see this comment of yours before posting mine, so apologies for the noise)
I think bumping minimum required version of MW is fine as long as there's something 1.35-compatible; for the things I maintain (incl. the social tools family of extensions) I aim for "master version of the extension/skin is compatible with LTS (previously latest stable) MW" because keeping up with the branched versions and whatnot is hard and generally speaking not worth it (for me; YMMV, obviously!).
But at this point I'm fine with whatever approach, as long as 1) we can have a version of GlobalUserrights that works w/ MW 1.35 and 2) ideally also have a version that won't explode when we upgrade from 1.35 to the next LTS release. ;)
It's a rather old and feature-complete extension (well, save for one silly hook, implemented on ShoutWiki as a one-liner core hack to the Special:Statistics code with the main code living in GlobalUserrights -- it's just responsible for displaying accurate stats on that special page for global groups like staff etc.) so I don't forsee any active feature development for it, just minimal maintenance work to ensure it keeps functioning as intended despite core MW changes.
Comment Actions
@ashley Thanks, this helps. My suggestion would be to bump the requirement to MW 1.38+ and just copy the missing pieces from UserrightsPage (similar to r738574). The REL1_37 version of the extension would still be compatible with MW 1.35. Otherwise you can check that the copied code works across all supported versions, but that's harder.