Error
MediaWiki version: 1.34.0-wmf.23
Feature MinervaTalkAtTop is not defined.
MediaWiki version: 1.34.0-wmf.23
Feature MinervaTalkAtTop is not defined.
#0 /extensions/MobileFrontend/includes/features/FeaturesManager.php(102): MobileFrontend\Features\FeaturesManager->getFeature(string) #1 /skins/MinervaNeue/includes/MinervaHooks.php(256): MobileFrontend\Features\FeaturesManager->isFeatureAvailableForCurrentUser(string) #2 /skins/MinervaNeue/includes/MinervaHooks.php(297): MinervaHooks::setMinervaSkinOptions(MobileContext, SkinMinerva) #3 /includes/Hooks.php(174): MinervaHooks::onUserLogoutComplete(User, string, string) #4 /includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL) #5 /includes/specials/SpecialUserLogout.php(107): Hooks::run(string, array) #6 /includes/specialpage/FormSpecialPage.php(186): SpecialUserLogout->onSuccess() #7 /includes/specials/SpecialUserLogout.php(61): FormSpecialPage->execute(NULL) #8 /includes/specialpage/SpecialPage.php(575): SpecialUserLogout->execute(NULL) #9 /includes/specialpage/SpecialPageFactory.php(611): SpecialPage->run(NULL) #10 /includes/MediaWiki.php(296): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext) #11 /includes/MediaWiki.php(892): MediaWiki->performRequest() #12 /includes/MediaWiki.php(523): MediaWiki->main() #13 /index.php(44): MediaWiki->run() #14 /srv/mediawiki/w/index.php(3): require(string) #15 {main}
Subject | Repo | Branch | Lines +/- | |
---|---|---|---|---|
Register all features immediately, not on RequestContextCreateSkin | mediawiki/extensions/MobileFrontend | master | +18 -33 |
This will occur if the UserLogoutComplete hook runs before RequestContextCreateSkin - not entirely sure when this might occur.
The issue is present in testwiki wmf.25 too.
I can consistently reproduce the issue in betalabs (with AMC enabled, not sure if it has some effect).
The Internal error screen will be present with the exception trace.
[XZPMugpAADwAAEHpIMAAAAAA] /wiki/Special:UserLogout RuntimeException from line 90 of /srv/mediawiki/php-1.34.0-wmf.25/extensions/MobileFrontend/includes/features/FeaturesManager.php: Feature MinervaTalkAtTop is not defined. Backtrace: #0 /srv/mediawiki/php-1.34.0-wmf.25/extensions/MobileFrontend/includes/features/FeaturesManager.php(102): MobileFrontend\Features\FeaturesManager->getFeature(string) #1 /srv/mediawiki/php-1.34.0-wmf.25/skins/MinervaNeue/includes/MinervaHooks.php(203): MobileFrontend\Features\FeaturesManager->isFeatureAvailableForCurrentUser(string) #2 /srv/mediawiki/php-1.34.0-wmf.25/skins/MinervaNeue/includes/MinervaHooks.php(244): MinervaHooks::setMinervaSkinOptions(MobileContext, SkinMinerva) #3 /srv/mediawiki/php-1.34.0-wmf.25/includes/Hooks.php(174): MinervaHooks::onUserLogoutComplete(User, string, string) #4 /srv/mediawiki/php-1.34.0-wmf.25/includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL) #5 /srv/mediawiki/php-1.34.0-wmf.25/includes/specials/SpecialUserLogout.php(107): Hooks::run(string, array) #6 /srv/mediawiki/php-1.34.0-wmf.25/includes/specialpage/FormSpecialPage.php(186): SpecialUserLogout->onSuccess() #7 /srv/mediawiki/php-1.34.0-wmf.25/includes/specials/SpecialUserLogout.php(61): FormSpecialPage->execute(NULL) #8 /srv/mediawiki/php-1.34.0-wmf.25/includes/specialpage/SpecialPage.php(575): SpecialUserLogout->execute(NULL) #9 /srv/mediawiki/php-1.34.0-wmf.25/includes/specialpage/SpecialPageFactory.php(611): SpecialPage->run(NULL) #10 /srv/mediawiki/php-1.34.0-wmf.25/includes/MediaWiki.php(296): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext) #11 /srv/mediawiki/php-1.34.0-wmf.25/includes/MediaWiki.php(900): MediaWiki->performRequest() #12 /srv/mediawiki/php-1.34.0-wmf.25/includes/MediaWiki.php(527): MediaWiki->main() #13 /srv/mediawiki/php-1.34.0-wmf.25/index.php(44): MediaWiki->run() #14 /srv/mediawiki/w/index.php(3): require(string) #15 {main}
Change 540836 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@master] Register all features immediately, not on RequestContextCreateSkin
So the problem is:
This issue happens only for desktop users who use Minerva as their default desktop skin. The solution is simple, register all features on first FeatureManager service access.
The proper fix is to move the FeatureManager from MobileFrontend to somwhere else (core/BetaFeatures ?) so it's coupled with MobileFrontend.
Thanks for investigating.
because we use Minerva as desktop skin, the mobile mode doesn't kick in, thus code from RequestContextCreateSkin is not called, therefore Minerva doesn't register it's features
That doesn't sound right to me. The RequestContextCreateSkin hook is called for all skins. We use RequestContextCreateSkinMobile in Minerva which is only run in the mobile domain.
Given the contents of that hook, I think we just need to turn onRequestContextCreateSkinMobile into onRequestContextCreateSkin and create an if statement wrapping it to check MobileFrontend extension is installed.
Does that make sense?
The problem is not Minerva, the problem is how/when MobileFrontend initializes FeatureManager. Just changing hook from RequestContextCreateSkinMobile to RequestContextCreateSkin won't help. The problem will be the same - The FeatureManager won't be initialized (won't send the hook).
So now the FeatureManager listens to RequestContextCreateSkin and when that happens it initializes FeatureManager (calls the hook to register features). If MobileFrontend doesn't come into play in that single request, then the FeatureManager won't be initialized (this line won't be called: https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileFrontendHooks.php#L97)
So now, the FeatureManager wasn't initialized (because it's desktop skin). During MinervaHooks::onLogoutComplete we reset the SkinOptions assuming that FeatureManager is registered. That's why it fails.
There are 3 solutions to this problem:
I went with the first approach to fix it properly. The second fix is nice and easy, should work, but we might encounter similar feature sometime in the future. It's better to protect future us from today's lazy us.
Another thing that is necessary to fix is to make MinervaHooks::setMinervaSkinOptions immune to the situation when MobileFrontend is not present (thus MobileFrontend.FeatureManager is not present).
Change 540836 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Register all features immediately, not on RequestContextCreateSkin