From c7b60735feb9e9bc642f57cc825a710b6cc26e4b Mon Sep 17 00:00:00 2001 From: Adam Wight Date: Fri, 6 Sep 2024 14:21:58 +0200 Subject: [PATCH] Move Reference Previews user preference into the Cite extension This seems to play well with Popups with and without Ie8fa1672b9fd . However, it's not clear to me why this still works and even gives priority to the Popups implementation when present, regardless of the order the extensions are loaded in. Happily, this is the desired behavior. Bug: T363162 Change-Id: Ic479c0a381ee16d1abcecfdd5ee48f0afccc1d3f --- extension.json | 5 +- i18n/en.json | 6 +- i18n/qqq.json | 6 +- src/Hooks/CiteHooks.php | 53 ++++++++++++++ .../ReferencePreviewsContext.php | 6 +- .../ReferencePreviewsGadgetsIntegration.php | 10 +-- src/ServiceWiring.php | 11 +++ tests/phpunit/CiteHooksTest.php | 70 +++++++++++++++++++ .../ReferencePreviewsContextTest.php | 2 + ...eferencePreviewsGadgetsIntegrationTest.php | 16 +++-- tests/phpunit/unit/CiteHooksUnitTest.php | 2 + 11 files changed, 167 insertions(+), 20 deletions(-) diff --git a/extension.json b/extension.json index 5acc10517..2eb3a7f28 100644 --- a/extension.json +++ b/extension.json @@ -26,6 +26,7 @@ "Hooks": { "APIQuerySiteInfoGeneralInfo": "main", "ContentHandlerDefaultModelFor": "main", + "GetPreferences": "main", "ParserAfterParse": "parser", "ParserClearState": "parser", "ParserCloned": "parser", @@ -33,13 +34,15 @@ "EditPage::showEditForm:initial": "main", "MakeGlobalVariablesScript": "main", "ResourceLoaderGetConfigVars": "main", - "ResourceLoaderRegisterModules": "main" + "ResourceLoaderRegisterModules": "main", + "UserGetDefaultOptions": "main" }, "HookHandlers": { "main": { "class": "Cite\\Hooks\\CiteHooks", "services": [ "Cite.ReferencePreviewsContext", + "Cite.GadgetsIntegration", "UserOptionsLookup" ] }, diff --git a/i18n/en.json b/i18n/en.json index 1e754285f..cc99fc3ff 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -79,5 +79,9 @@ "cite-wikieditor-help-content-reference-example-ref-reuse": "", "cite-wikieditor-help-content-reference-example-ref-extends": "$2", "cite-wikieditor-help-content-reference-example-ref-result": "[$1]", - "cite-wikieditor-help-content-reference-example-reflist": "" + "cite-wikieditor-help-content-reference-example-reflist": "", + "popups-prefs-navpopups-gadget-conflict-info": "You have the [[$1|Navigation popups]] gadget enabled, so you won't see previews provided by this feature. Depending on your wiki, the gadget may have a slightly different name. If you continue to experience issues, please review your gadgets and user scripts, including global ones.", + "popups-prefs-reftooltips-and-navpopups-gadget-conflict-info": "You have the [[$1|Navigation popups]] and [[$1|Reference Tooltips]] gadgets enabled, so you won't see previews provided by this feature. Depending on your wiki, the gadgets may have slightly different names. If you continue to experience issues, please review your gadgets and user scripts, including global ones.", + "popups-prefs-reftooltips-gadget-conflict-info": "You have the [[$1|Reference Tooltips]] gadget enabled, so you won't see reference previews but will still see page previews. Depending on your wiki, the gadget may have a slightly different name. If you continue to experience issues, please review your gadgets and user scripts, including global ones.", + "popups-refpreview-user-preference-label": "Enable reference previews (get quick previews of a reference while reading a page)" } diff --git a/i18n/qqq.json b/i18n/qqq.json index cfe49e759..8a05d394f 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -84,5 +84,9 @@ "cite-wikieditor-help-content-reference-example-ref-reuse": "{{ignored}}", "cite-wikieditor-help-content-reference-example-ref-extends": "{{ignored}}", "cite-wikieditor-help-content-reference-example-ref-result": "{{ignored}}", - "cite-wikieditor-help-content-reference-example-reflist": "{{ignored}}" + "cite-wikieditor-help-content-reference-example-reflist": "{{ignored}}", + "popups-prefs-navpopups-gadget-conflict-info": "Help message telling to disable the \"Navigation popups\" gadget in order to allow page and reference previews. The word \"Gadgets\" should be based on {{msg-mw|prefs-gadgets}}.\n\nParameters:\n* $1 – Link to the Gadgets tab in the user's preferences", + "popups-prefs-reftooltips-gadget-conflict-info": "Help message telling to disable the \"Reference Tooltips\" gadget in order to allow reference previews. The word \"Gadgets\" should be based on {{msg-mw|prefs-gadgets}}.\n\nParameters:\n* $1 – Link to the Gadgets tab in the user's preferences", + "popups-prefs-reftooltips-and-navpopups-gadget-conflict-info": "Help message telling to disable the \"Navigation popups\" and \"Reference Tooltips\" gadgets in order to allow page and reference previews. The word \"Gadgets\" should be based on {{msg-mw|prefs-gadgets}}.\n\nParameters:\n* $1 – Link to the Gadgets tab in the user's preferences", + "popups-refpreview-user-preference-label": "Label for the user option to enable or disable reference preview popups" } diff --git a/src/Hooks/CiteHooks.php b/src/Hooks/CiteHooks.php index cfa49e34f..b3ec36552 100644 --- a/src/Hooks/CiteHooks.php +++ b/src/Hooks/CiteHooks.php @@ -8,6 +8,7 @@ namespace Cite\Hooks; use ApiQuerySiteinfo; use Cite\ReferencePreviews\ReferencePreviewsContext; +use Cite\ReferencePreviews\ReferencePreviewsGadgetsIntegration; use ExtensionRegistry; use MediaWiki\Api\Hook\APIQuerySiteInfoGeneralInfoHook; use MediaWiki\Config\Config; @@ -21,6 +22,7 @@ use MediaWiki\ResourceLoader\ResourceLoader; use MediaWiki\Revision\Hook\ContentHandlerDefaultModelForHook; use MediaWiki\Title\Title; use MediaWiki\User\Options\UserOptionsLookup; +use MediaWiki\User\User; /** * @license GPL-2.0-or-later @@ -36,13 +38,16 @@ class CiteHooks implements { private ReferencePreviewsContext $referencePreviewsContext; + private ReferencePreviewsGadgetsIntegration $gadgetsIntegration; private UserOptionsLookup $userOptionsLookup; public function __construct( ReferencePreviewsContext $referencePreviewsContext, + ReferencePreviewsGadgetsIntegration $gadgetsIntegration, UserOptionsLookup $userOptionsLookup ) { $this->referencePreviewsContext = $referencePreviewsContext; + $this->gadgetsIntegration = $gadgetsIntegration; $this->userOptionsLookup = $userOptionsLookup; } @@ -172,4 +177,52 @@ class CiteHooks implements } } + /** + * Add options to user Preferences page + * + * @param User $user User whose preferences are being modified + * @param array[] &$prefs Preferences description array, to be fed to a HTMLForm object + */ + public function onGetPreferences( $user, &$prefs ) { + $option = [ + 'type' => 'toggle', + 'label-message' => 'popups-refpreview-user-preference-label', + // FIXME: This message is unnecessary and unactionable since we already + // detect specific gadget conflicts. + 'help-message' => 'popups-prefs-conflicting-gadgets-info', + // FIXME: copied from Popups + 'section' => 'rendering/reading', + ]; + $isNavPopupsGadgetEnabled = $this->gadgetsIntegration->isNavPopupsGadgetEnabled( $user ); + $isRefTooltipsGadgetEnabled = $this->gadgetsIntegration->isRefTooltipsGadgetEnabled( $user ); + if ( $isNavPopupsGadgetEnabled && $isRefTooltipsGadgetEnabled ) { + $option[ 'disabled' ] = true; + $option[ 'help-message' ] = [ 'popups-prefs-reftooltips-and-navpopups-gadget-conflict-info', + 'Special:Preferences#mw-prefsection-gadgets' ]; + } elseif ( $isNavPopupsGadgetEnabled ) { + $option[ 'disabled' ] = true; + $option[ 'help-message' ] = [ 'popups-prefs-navpopups-gadget-conflict-info', + 'Special:Preferences#mw-prefsection-gadgets' ]; + } elseif ( $isRefTooltipsGadgetEnabled ) { + $option[ 'disabled' ] = true; + $option[ 'help-message' ] = [ 'popups-prefs-reftooltips-gadget-conflict-info', + 'Special:Preferences#mw-prefsection-gadgets' ]; + } + + $prefs += [ + ReferencePreviewsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME => $option + ]; + } + + /** + * See https://www.mediawiki.org/wiki/Manual:Hooks/UserGetDefaultOptions + * @param array &$defaultOptions Array of preference keys and their default values. + */ + public static function onUserGetDefaultOptions( &$defaultOptions ) { + // FIXME: Move to extension.json once migration is complete. See T363162 + if ( !isset( $defaultOptions[ ReferencePreviewsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME ] ) ) { + $defaultOptions[ ReferencePreviewsContext::REFERENCE_PREVIEWS_PREFERENCE_NAME ] = '1'; + } + } + } diff --git a/src/ReferencePreviews/ReferencePreviewsContext.php b/src/ReferencePreviews/ReferencePreviewsContext.php index bc7e992aa..ef9134739 100644 --- a/src/ReferencePreviews/ReferencePreviewsContext.php +++ b/src/ReferencePreviews/ReferencePreviewsContext.php @@ -18,12 +18,12 @@ class ReferencePreviewsContext { public function __construct( Config $config, + ReferencePreviewsGadgetsIntegration $gadgetsIntegration, UserOptionsLookup $userOptionsLookup ) { - $this->gadgetsIntegration = new ReferencePreviewsGadgetsIntegration( $config ); - $this->userOptionsLookup = $userOptionsLookup; - $this->config = $config; + $this->gadgetsIntegration = $gadgetsIntegration; + $this->userOptionsLookup = $userOptionsLookup; } /** diff --git a/src/ReferencePreviews/ReferencePreviewsGadgetsIntegration.php b/src/ReferencePreviews/ReferencePreviewsGadgetsIntegration.php index c38315a9b..befa6fb9b 100644 --- a/src/ReferencePreviews/ReferencePreviewsGadgetsIntegration.php +++ b/src/ReferencePreviews/ReferencePreviewsGadgetsIntegration.php @@ -5,10 +5,8 @@ namespace Cite\ReferencePreviews; use InvalidArgumentException; use MediaWiki\Config\Config; use MediaWiki\Extension\Gadgets\GadgetRepo; -use MediaWiki\MediaWikiServices; use MediaWiki\User\User; use MediaWiki\User\UserIdentity; -use Wikimedia\Services\NoSuchServiceException; /** * Gadgets integration @@ -25,17 +23,13 @@ class ReferencePreviewsGadgetsIntegration { private string $navPopupsGadgetName; private string $refTooltipsGadgetName; - public function __construct( Config $config ) { + public function __construct( Config $config, ?GadgetRepo $gadgetRepo ) { $this->navPopupsGadgetName = $this->sanitizeGadgetName( $config->get( self::CONFIG_NAVIGATION_POPUPS_NAME ) ); $this->refTooltipsGadgetName = $this->sanitizeGadgetName( $config->get( self::CONFIG_REFERENCE_TOOLTIPS_NAME ) ); - try { - $this->gadgetRepo = MediaWikiServices::getInstance()->getService( 'GadgetsRepo' ); - } catch ( NoSuchServiceException $e ) { - $this->gadgetRepo = null; - } + $this->gadgetRepo = $gadgetRepo; } private function sanitizeGadgetName( string $gadgetName ): string { diff --git a/src/ServiceWiring.php b/src/ServiceWiring.php index 99c243afe..cc1864b04 100644 --- a/src/ServiceWiring.php +++ b/src/ServiceWiring.php @@ -1,15 +1,26 @@ static function ( MediaWikiServices $services ): ReferencePreviewsGadgetsIntegration { + return new ReferencePreviewsGadgetsIntegration( + $services->getMainConfig(), + ExtensionRegistry::getInstance()->isLoaded( 'Gadgets' ) ? + $services->getService( 'GadgetsRepo' ) : + null + ); + }, 'Cite.ReferencePreviewsContext' => static function ( MediaWikiServices $services ): ReferencePreviewsContext { return new ReferencePreviewsContext( $services->getMainConfig(), + $services->getService( 'Cite.GadgetsIntegration' ), $services->getUserOptionsLookup() ); }, diff --git a/tests/phpunit/CiteHooksTest.php b/tests/phpunit/CiteHooksTest.php index 0ced36b06..0c127e4ce 100644 --- a/tests/phpunit/CiteHooksTest.php +++ b/tests/phpunit/CiteHooksTest.php @@ -4,9 +4,11 @@ namespace Cite\Tests; use ApiQuerySiteinfo; use Cite\Hooks\CiteHooks; +use Cite\ReferencePreviews\ReferencePreviewsGadgetsIntegration; use MediaWiki\Config\HashConfig; use MediaWiki\ResourceLoader\ResourceLoader; use MediaWiki\User\Options\StaticUserOptionsLookup; +use MediaWiki\User\User; /** * @covers \Cite\Hooks\CiteHooks @@ -28,6 +30,7 @@ class CiteHooksTest extends \MediaWikiIntegrationTestCase { ( new CiteHooks( $this->getServiceContainer()->getService( 'Cite.ReferencePreviewsContext' ), + $this->getServiceContainer()->getService( 'Cite.GadgetsIntegration' ), new StaticUserOptionsLookup( [] ) ) ) ->onResourceLoaderGetConfigVars( $vars, 'vector', $config ); @@ -53,6 +56,7 @@ class CiteHooksTest extends \MediaWikiIntegrationTestCase { ( new CiteHooks( $this->getServiceContainer()->getService( 'Cite.ReferencePreviewsContext' ), + $this->getServiceContainer()->getService( 'Cite.GadgetsIntegration' ), new StaticUserOptionsLookup( [] ) ) ) ->onResourceLoaderRegisterModules( $resourceLoader ); @@ -71,6 +75,7 @@ class CiteHooksTest extends \MediaWikiIntegrationTestCase { ( new CiteHooks( $this->getServiceContainer()->getService( 'Cite.ReferencePreviewsContext' ), + $this->getServiceContainer()->getService( 'Cite.GadgetsIntegration' ), new StaticUserOptionsLookup( [] ) ) ) ->onAPIQuerySiteInfoGeneralInfo( $api, $data ); @@ -83,4 +88,69 @@ class CiteHooksTest extends \MediaWikiIntegrationTestCase { yield [ false ]; } + public function testOnGetPreferences_noConflicts() { + $expected = [ + 'popups-reference-previews' => [ + 'type' => 'toggle', + 'label-message' => 'popups-refpreview-user-preference-label', + 'help-message' => 'popups-prefs-conflicting-gadgets-info', + 'section' => 'rendering/reading' + ] + ]; + $gadgetsIntegrationMock = $this->createMock( ReferencePreviewsGadgetsIntegration::class ); + $prefs = []; + ( new CiteHooks( + $this->getServiceContainer()->getService( 'Cite.ReferencePreviewsContext' ), + $gadgetsIntegrationMock, + new StaticUserOptionsLookup( [] ) + ) ) + ->onGetPreferences( $this->createMock( User::class ), $prefs ); + $this->assertEquals( $expected, $prefs ); + } + + public function testOnGetPreferences_conflictingGadget() { + $expected = [ + 'popups-reference-previews' => [ + 'type' => 'toggle', + 'label-message' => 'popups-refpreview-user-preference-label', + // 'help-message' => 'popups-prefs-conflicting-gadgets-info', + 'help-message' => [ + 'popups-prefs-navpopups-gadget-conflict-info', + 'Special:Preferences#mw-prefsection-gadgets', + ], + 'section' => 'rendering/reading', + 'disabled' => true + ] + ]; + $gadgetsIntegrationMock = $this->createMock( ReferencePreviewsGadgetsIntegration::class ); + $gadgetsIntegrationMock->expects( $this->once() ) + ->method( 'isNavPopupsGadgetEnabled' ) + ->willReturn( true ); + $prefs = []; + ( new CiteHooks( + $this->getServiceContainer()->getService( 'Cite.ReferencePreviewsContext' ), + $gadgetsIntegrationMock, + new StaticUserOptionsLookup( [] ) + ) ) + ->onGetPreferences( $this->createMock( User::class ), $prefs ); + $this->assertEquals( $expected, $prefs ); + } + + public function testOnGetPreferences_redundantPreference() { + $prefs = [ + 'popups-reference-previews' => [ + 'type' => 'toggle', + 'label-message' => 'from-another-extension', + ] + ]; + $expected = $prefs; + ( new CiteHooks( + $this->getServiceContainer()->getService( 'Cite.ReferencePreviewsContext' ), + $this->getServiceContainer()->getService( 'Cite.GadgetsIntegration' ), + new StaticUserOptionsLookup( [] ) + ) ) + ->onGetPreferences( $this->createMock( User::class ), $prefs ); + $this->assertEquals( $expected, $prefs ); + } + } diff --git a/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsContextTest.php b/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsContextTest.php index 69b196d84..fe603e48f 100644 --- a/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsContextTest.php +++ b/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsContextTest.php @@ -40,6 +40,7 @@ class ReferencePreviewsContextTest extends MediaWikiIntegrationTestCase { $this->assertSame( $expected, ( new ReferencePreviewsContext( $config, + $this->getServiceContainer()->getService( 'Cite.GadgetsIntegration' ), $userOptLookup ) ) ->isReferencePreviewsEnabled( $user, $skin ), @@ -80,6 +81,7 @@ class ReferencePreviewsContextTest extends MediaWikiIntegrationTestCase { $this->assertSame( $expected, ( new ReferencePreviewsContext( $config, + $this->getServiceContainer()->getService( 'Cite.GadgetsIntegration' ), $userOptLookup ) ) ->isReferencePreviewsEnabled( $user, $skin ), diff --git a/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsGadgetsIntegrationTest.php b/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsGadgetsIntegrationTest.php index 6d4867bb3..68cffaeaa 100644 --- a/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsGadgetsIntegrationTest.php +++ b/tests/phpunit/integration/ReferencePreviews/ReferencePreviewsGadgetsIntegrationTest.php @@ -39,9 +39,12 @@ class ReferencePreviewsGadgetsIntegrationTest extends MediaWikiIntegrationTestCa } public function testConflictsWithNavPopupsGadgetIfGadgetsExtensionIsNotLoaded() { - $integration = new ReferencePreviewsGadgetsIntegration( $this->getConfig() ); $this->assertFalse( - $integration->isNavPopupsGadgetEnabled( $this->createNoOpMock( User::class ) ), + ( new ReferencePreviewsGadgetsIntegration( + $this->getConfig(), + null + ) ) + ->isNavPopupsGadgetEnabled( $this->createNoOpMock( User::class ) ), 'No conflict is identified.' ); } @@ -153,12 +156,13 @@ class ReferencePreviewsGadgetsIntegrationTest extends MediaWikiIntegrationTestCa GadgetRepo $repoMock, bool $expected ): void { - $this->setService( 'GadgetsRepo', $repoMock ); - - $integration = new ReferencePreviewsGadgetsIntegration( $config ); $this->assertSame( $expected, - $integration->isNavPopupsGadgetEnabled( $user ), + ( new ReferencePreviewsGadgetsIntegration( + $config, + $repoMock + ) ) + ->isNavPopupsGadgetEnabled( $user ), ( $expected ? 'A' : 'No' ) . ' conflict is identified.' ); } diff --git a/tests/phpunit/unit/CiteHooksUnitTest.php b/tests/phpunit/unit/CiteHooksUnitTest.php index a4817e8ae..82c4e0d74 100644 --- a/tests/phpunit/unit/CiteHooksUnitTest.php +++ b/tests/phpunit/unit/CiteHooksUnitTest.php @@ -4,6 +4,7 @@ namespace Cite\Tests\Unit; use Cite\Hooks\CiteHooks; use Cite\ReferencePreviews\ReferencePreviewsContext; +use Cite\ReferencePreviews\ReferencePreviewsGadgetsIntegration; use MediaWiki\Title\Title; use MediaWiki\User\Options\StaticUserOptionsLookup; @@ -22,6 +23,7 @@ class CiteHooksUnitTest extends \MediaWikiUnitTestCase { ( new CiteHooks( $this->createMock( ReferencePreviewsContext::class ), + $this->createMock( ReferencePreviewsGadgetsIntegration::class ), new StaticUserOptionsLookup( [] ) ) ) ->onContentHandlerDefaultModelFor( $title, $model );