jenkins-bot has submitted this change and it was merged.

Change subject: Fix unstyled "edit links" when page has noexternallanglinks
......................................................................


Fix unstyled "edit links" when page has noexternallanglinks

If a page was connected to wikibase and had links,
then we were not properly checking noexternallanglinks.

RepoItemLinkGeneratorTest wasn't properly testing this.
Fixed and added more test cases.

Also, having "edit links" when noexternallanglinks suppresses
only some links and not all was not handled consistently.

Bug: T94889
Change-Id: I4877e62b71ed2027762627d8228617d9ba6a47f9
(cherry picked from commit 39c9fe476b18531ad8051831ba28a7467b63b845)
---
M client/includes/Hooks/BeforePageDisplayHandler.php
M client/includes/RepoItemLinkGenerator.php
M client/tests/phpunit/includes/Hooks/BeforePageDisplayHandlerTest.php
M client/tests/phpunit/includes/RepoItemLinkGeneratorTest.php
4 files changed, 52 insertions(+), 12 deletions(-)

Approvals:
  Aude: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/client/includes/Hooks/BeforePageDisplayHandler.php 
b/client/includes/Hooks/BeforePageDisplayHandler.php
index b3e0d44..68b2336 100644
--- a/client/includes/Hooks/BeforePageDisplayHandler.php
+++ b/client/includes/Hooks/BeforePageDisplayHandler.php
@@ -67,8 +67,8 @@
 
        private function hasEditOrAddLinks( OutputPage $out, Title $title, 
$actionName ) {
                if (
-                       $out->getProperty( 'noexternallanglinks' ) ||
                        !in_array( $actionName, array( 'view', 'submit' ) ) ||
+                       $this->allLinksAreSuppressed( $out ) ||
                        !$title->exists()
                ) {
                        return false;
@@ -77,6 +77,16 @@
                return true;
        }
 
+       private function allLinksAreSuppressed( OutputPage $out ) {
+               $noexternallanglinks = $out->getProperty( 'noexternallanglinks' 
);
+
+               if ( $noexternallanglinks !== null ) {
+                       return in_array( '*', $noexternallanglinks );
+               }
+
+               return false;
+       }
+
        private function hasLinkItemWidget( User $user, OutputPage $out, Title 
$title, $actionName ) {
                if (
                        $out->getLanguageLinks() !== array() || 
!$user->isLoggedIn()
diff --git a/client/includes/RepoItemLinkGenerator.php 
b/client/includes/RepoItemLinkGenerator.php
index 90bac80..1e765cc 100644
--- a/client/includes/RepoItemLinkGenerator.php
+++ b/client/includes/RepoItemLinkGenerator.php
@@ -77,16 +77,17 @@
         * @return string|null HTML or null for no link
         */
        public function getLink( Title $title, $action, $hasLangLinks, array 
$noExternalLangLinks = null, $prefixedId ) {
-               $entityId = null;
-               if ( is_string( $prefixedId ) ) {
-                       $entityId = $this->entityIdParser->parse( $prefixedId );
-               }
-
-               if ( $entityId && $hasLangLinks ) {
-                       return $this->getEditLinksLink( $entityId );
-               }
-
                if ( $this->canHaveLink( $title, $action, $noExternalLangLinks 
) ) {
+                       $entityId = null;
+
+                       if ( is_string( $prefixedId ) ) {
+                               $entityId = $this->entityIdParser->parse( 
$prefixedId );
+                       }
+
+                       if ( $entityId && $hasLangLinks ) {
+                               return $this->getEditLinksLink( $entityId );
+                       }
+
                        return $this->getAddLinksLink( $title, $entityId );
                }
 
diff --git 
a/client/tests/phpunit/includes/Hooks/BeforePageDisplayHandlerTest.php 
b/client/tests/phpunit/includes/Hooks/BeforePageDisplayHandlerTest.php
index 6d91360..9932a60 100644
--- a/client/tests/phpunit/includes/Hooks/BeforePageDisplayHandlerTest.php
+++ b/client/tests/phpunit/includes/Hooks/BeforePageDisplayHandlerTest.php
@@ -89,6 +89,20 @@
                );
        }
 
+       public function testHandlePageConnectedToWikibase_noexternallinklinks() 
{
+               $skin = $this->getSkin( true, true ); // user logged in
+
+               // page connected, has links and noexternallanglinks
+               $output = $this->getOutputPage( $skin, array( 'de:Rom' ), 'Q4', 
array( '*' ) );
+               $namespaceChecker = $this->getNamespaceChecker( true );
+
+               $handler = new BeforePageDisplayHandler( $namespaceChecker );
+               $handler->addModules( $output, 'view' );
+
+               $this->assertEquals( array(), $output->getModules(), 'js 
modules' );
+               $this->assertEquals( array(), $output->getModuleStyles(), 'css 
modules' );
+       }
+
        /**
         * @dataProvider pageNotConnectedToWikibaseProvider
         */
@@ -261,7 +275,9 @@
                return $namespaceChecker;
        }
 
-       private function getOutputPage( Skin $skin, $langLinks, $prefixedId = 
null ) {
+       private function getOutputPage( Skin $skin, $langLinks, $prefixedId = 
null,
+               $noexternallanglinks = null
+       ) {
                $output = $skin->getOutput();
                $output->setLanguageLinks( $langLinks );
 
@@ -269,6 +285,10 @@
                        $output->setProperty( 'wikibase_item', $prefixedId );
                }
 
+               if ( $noexternallanglinks ) {
+                       $output->setProperty( 'noexternallanglinks', 
$noexternallanglinks );
+               }
+
                return $output;
        }
 
diff --git a/client/tests/phpunit/includes/RepoItemLinkGeneratorTest.php 
b/client/tests/phpunit/includes/RepoItemLinkGeneratorTest.php
index cdb6da0..1d5c3fd 100644
--- a/client/tests/phpunit/includes/RepoItemLinkGeneratorTest.php
+++ b/client/tests/phpunit/includes/RepoItemLinkGeneratorTest.php
@@ -112,6 +112,15 @@
                        'hasLangLinks' => true
                );
 
+               $data['edit link when had links and suppressing one link'] = 
array(
+                       'expected' => $editLinksLinkRegex,
+                       'title' => $title,
+                       'action' => 'view',
+                       'noExternalLangLinks' => array( 'fr' ),
+                       'prefixedId' => $prefixedId,
+                       'hasLangLinks' => true
+               );
+
                $data['title does not exist'] = array(
                        'expected' => null,
                        'title' => $nonExistingTitle,
@@ -153,7 +162,7 @@
                );
 
                if ( $expected === null ) {
-                       $this->assertNull( $expected );
+                       $this->assertNull( $link );
                } else {
                        $this->assertRegexp( $expected, $link );
                }

-- 
To view, visit https://gerrit.wikimedia.org/r/202009
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4877e62b71ed2027762627d8228617d9ba6a47f9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: wmf/1.25wmf24
Gerrit-Owner: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to