Use subtitle to modify the skin template (#47)

SemanticMediaWiki · Sep 9, 2018 · ea14c41 · ea14c41
1 parent 712a1bc
commit ea14c41
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 72 deletions.
diff --git a/res/sbl.styles.css b/res/sbl.styles.css
@@ -77,6 +77,14 @@
 	margin: .3em 0 0 0.6em;
 }
 
+/**
+ * @see the contentSub CSS, substract -1em to start from 0
+ */
+#contentSub #sbl-breadcrumbs {
+	font-size: 100%;
+	margin: 0 0 1.4em -1em;
+}
+
 /* SBL chameleon */
 .skin-chameleon .sbl-breadcrumb-children-list {
     margin: .1em 0 0 -1.5em;

diff --git a/src/ByPropertyHierarchicalLinksFinder.php b/src/ByPropertyHierarchicalLinksFinder.php
@@ -83,6 +83,7 @@ public function findLinksBySubject( DIWikiPage $subject ) {
 
 		$requestOptions = new RequestOptions();
 		$requestOptions->sort = true;
+		$requestOptions->conditionConstraint = true;
 
 		// Use 3 as buffer to broaden match possibilities
 		$requestOptions->limit = 3;

diff --git a/src/BySubpageLinksFinder.php b/src/BySubpageLinksFinder.php
@@ -48,13 +48,17 @@ public function isDiscoveryFallback() {
 	 */
 	public function findLinksBySubject( DIWikiPage $subject ) {
 
-		$prefixedText = $subject->getTitle()->getPrefixedText();
+		$title = $subject->getTitle();
 
-		if ( !$this->canBuildLinksFromText( $prefixedText ) ) {
+		// Use the text instead of the prefixedText to avoid a split
+		// in cases where the NS contains a / (e.g. smw/schema:Foobar)
+		$text = $title->getText();
+
+		if ( !$this->canBuildLinksFromText( $text ) ) {
 			return;
 		}
 
-		$this->buildHierarchicalLinksFromText( $prefixedText );
+		$this->buildHierarchicalLinksFromText( $text, $title->getNamespace() );
 	}
 
 	/**
@@ -70,7 +74,7 @@ private function canBuildLinksFromText( $text ) {
 		return preg_match( '/\//', $text );
 	}
 
-	private function buildHierarchicalLinksFromText( $text ) {
+	private function buildHierarchicalLinksFromText( $text, $ns ) {
 
 		$growinglink = '';
 		$links = explode( '/', $text );
@@ -82,7 +86,7 @@ private function buildHierarchicalLinksFromText( $text ) {
 
 			if ( $link !== '' && substr( $link, -1 ) !== ' ' ) {
 				$growinglink .= $link;
-				$this->antecedentHierarchyLinks[] = DIWikiPage::newFromTitle( Title::newFromText( $growinglink ) );
+				$this->antecedentHierarchyLinks[] = DIWikiPage::newFromTitle( Title::newFromText( $growinglink, $ns ) );
 			}
 
 			$growinglink .= '/';

diff --git a/src/HookRegistry.php b/src/HookRegistry.php
@@ -133,9 +133,12 @@ private function addCallbackHandlers( $store, $options ) {
 				$options->get( 'hideSubpageParent' )
 			);
 
-			$skinTemplateOutputModifier = new SkinTemplateOutputModifier( $htmlBreadcrumbLinksBuilder );
-			$skinTemplateOutputModifier->modifyTemplate( $template );
-			$skinTemplateOutputModifier->modifyOutput( $skin->getOutput() );
+			$skinTemplateOutputModifier = new SkinTemplateOutputModifier(
+				$htmlBreadcrumbLinksBuilder,
+				ApplicationFactory::getInstance()->getNamespaceExaminer()
+			);
+
+			$skinTemplateOutputModifier->modify( $skin->getOutput(), $template );
 
 			return true;
 		};

diff --git a/src/SkinTemplateOutputModifier.php b/src/SkinTemplateOutputModifier.php
@@ -3,6 +3,7 @@
 namespace SBL;
 
 use SMW\ApplicationFactory;
+use SMW\NamespaceExaminer;
 use OutputPage;
 use Action;
 use Title;
@@ -20,36 +21,48 @@ class SkinTemplateOutputModifier {
 	 */
 	private $htmlBreadcrumbLinksBuilder;
 
+	/**
+	 * @var NnamespaceExaminer
+	 */
+	private $namespaceExaminer;
+
 	/**
 	 * @since 1.0
 	 *
 	 * @param HtmlBreadcrumbLinksBuilder $htmlBreadcrumbLinksBuilder
+	 * @param NamespaceExaminer $namespaceExaminer
 	 */
-	public function __construct( HtmlBreadcrumbLinksBuilder $htmlBreadcrumbLinksBuilder ) {
+	public function __construct( HtmlBreadcrumbLinksBuilder $htmlBreadcrumbLinksBuilder, NamespaceExaminer $namespaceExaminer ) {
 		$this->htmlBreadcrumbLinksBuilder = $htmlBreadcrumbLinksBuilder;
+		$this->namespaceExaminer = $namespaceExaminer;
 	}
 
 	/**
-	 * @since  1.0
+	 * @since 1.5
 	 *
 	 * @param OutputPage $output
-	 *
-	 * @return boolean
-	 */
-	public function modifyOutput( OutputPage $output ) {
-		return $this->canModifyOutput( $output ) ? $this->doModifyOutput( $output ) : true;
-	}
-
-	/**
-	 * @since  1.0
-	 *
 	 * @param &$template
 	 */
-	public function modifyTemplate( &$template ) {
+	public function modify( OutputPage $output, &$template ) {
+
+		if ( !$this->canModifyOutput( $output ) ) {
+			return;
+		}
+
+		$title = $output->getTitle();
+		$this->htmlBreadcrumbLinksBuilder->buildBreadcrumbs( $title );
+
+		$this->htmlBreadcrumbLinksBuilder->isRTL(
+			$title->getPageLanguage()->isRTL()
+		);
 
-		// Always set subtitle to be empty when SBL is used to avoid any output
-		// distraction
-		$template->data['subtitle'] = '';
+		if ( !isset( $template->data['subtitle'] ) ) {
+			$template->data['subtitle'] = '';
+		}
+
+		// We always assume `subtitle` is available!
+		// https://github.com/wikimedia/mediawiki/blob/23ea2e4c2966f381eb7fd69b66a8d738bb24cc60/includes/skins/SkinTemplate.php#L292-L296
+		$template->data['subtitle'] = $this->htmlBreadcrumbLinksBuilder->getHtml();
 	}
 
 	private function canModifyOutput( OutputPage $output ) {
@@ -58,34 +71,15 @@ private function canModifyOutput( OutputPage $output ) {
 			return false;
 		}
 
-		if ( Action::getActionName( $output->getContext() ) !== 'view' ) {
-			return false;
-		}
-
 		if ( isset( $output->smwmagicwords ) && in_array( 'SBL_NOBREADCRUMBLINKS', $output->smwmagicwords ) ) {
 			return false;
 		}
 
 		return true;
 	}
 
-	private function doModifyOutput( OutputPage $output ) {
-
-		$this->htmlBreadcrumbLinksBuilder->buildBreadcrumbs( $output->getTitle() );
-
-		$this->htmlBreadcrumbLinksBuilder->isRTL(
-			$output->getTitle()->getPageLanguage()->isRTL()
-		);
-
-		$output->prependHTML( $this->htmlBreadcrumbLinksBuilder->getHtml() );
-
-		return true;
-	}
-
 	private function isEnabled( Title $title ) {
-		return $title->isKnown() &&
-			!$title->isSpecialPage() &&
-			ApplicationFactory::getInstance()->getNamespaceExaminer()->isSemanticEnabled( $title->getNamespace() );
+		return $title->isKnown() && !$title->isSpecialPage() && $this->namespaceExaminer->isSemanticEnabled( $title->getNamespace() );
 	}
 
 }
diff --git a/tests/phpunit/Unit/BySubpageLinksFinderTest.php b/tests/phpunit/Unit/BySubpageLinksFinderTest.php
@@ -38,7 +38,7 @@ public function testDisabledFinder() {
 	/**
 	 * @dataProvider titleProvider
 	 */
-	public function testFindParentBreadcrumbs( $title,$count, $expected ) {
+	public function testFindParentBreadcrumbs( $title, $count, $expected ) {
 
 		$subject = DIWikiPage::newFromTitle( Title::newFromText( $title ) );
 
@@ -142,6 +142,15 @@ public function titleProvider() {
 			[]
 		];
 
+		#9
+		$provider[] = [
+			'Help:Foo/Foobar',
+			1,
+			[
+				new DIWikiPage( 'Foo', NS_HELP ),
+			]
+		];
+
 		return $provider;
 	}
 

diff --git a/tests/phpunit/Unit/SkinTemplateOutputModifierTest.php b/tests/phpunit/Unit/SkinTemplateOutputModifierTest.php
@@ -17,20 +17,33 @@
  */
 class SkinTemplateOutputModifierTest extends \PHPUnit_Framework_TestCase {
 
+	private $namespaceExaminer;
+
+	protected function setUp() {
+
+		$this->namespaceExaminer = $this->getMockBuilder( '\SMW\NamespaceExaminer' )
+			->disableOriginalConstructor()
+			->getMock();
+	}
+
 	public function testCanConstruct() {
 
 		$htmlBreadcrumbLinksBuilder = $this->getMockBuilder( '\SBL\HtmlBreadcrumbLinksBuilder' )
 			->disableOriginalConstructor()
 			->getMock();
 
 		$this->assertInstanceOf(
-			'\SBL\SkinTemplateOutputModifier',
-			new SkinTemplateOutputModifier( $htmlBreadcrumbLinksBuilder )
+			SkinTemplateOutputModifier::class,
+			new SkinTemplateOutputModifier( $htmlBreadcrumbLinksBuilder, $this->namespaceExaminer )
 		);
 	}
 
 	public function testTryPrependHtmlOnUnknownTitle() {
 
+		$this->namespaceExaminer->expects( $this->never() )
+			->method( 'isSemanticEnabled' )
+			->will( $this->returnValue( true ) );
+
 		$htmlBreadcrumbLinksBuilder = $this->getMockBuilder( '\SBL\HtmlBreadcrumbLinksBuilder' )
 			->disableOriginalConstructor()
 			->getMock();
@@ -50,17 +63,19 @@ public function testTryPrependHtmlOnUnknownTitle() {
 		$output->expects( $this->never() )
 			->method( 'prependHTML' );
 
-		$output->expects( $this->once() )
+		$output->expects( $this->atLeastOnce() )
 			->method( 'getTitle' )
 			->will( $this->returnValue( $title ) );
 
 		$instance = new SkinTemplateOutputModifier(
-			$htmlBreadcrumbLinksBuilder
+			$htmlBreadcrumbLinksBuilder,
+			$this->namespaceExaminer
 		);
 
-		$this->assertTrue(
-			$instance->modifyOutput( $output )
-		);
+		$template = new \stdClass;
+		$template->data = [];
+
+		$instance->modify( $output, $template );
 	}
 
 	public function testTryPrependHtmlOnSpecialPage() {
@@ -93,15 +108,17 @@ public function testTryPrependHtmlOnSpecialPage() {
 			->will( $this->returnValue( $title ) );
 
 		$instance = new SkinTemplateOutputModifier(
-			$htmlBreadcrumbLinksBuilder
+			$htmlBreadcrumbLinksBuilder,
+			$this->namespaceExaminer
 		);
 
-		$this->assertTrue(
-			$instance->modifyOutput( $output )
-		);
+		$template = new \stdClass;
+		$template->data = [];
+
+		$instance->modify( $output, $template );
 	}
 
-	public function testTryPrependHtmlOnNonViewAction() {
+	public function PrependHtmlOnNonViewAction() {
 
 		$context = new \RequestContext();
 		$context->setRequest( new \FauxRequest( [ 'action' => 'edit' ], true ) );
@@ -143,12 +160,14 @@ public function testTryPrependHtmlOnNonViewAction() {
 			->will( $this->returnValue( $title ) );
 
 		$instance = new SkinTemplateOutputModifier(
-			$htmlBreadcrumbLinksBuilder
+			$htmlBreadcrumbLinksBuilder,
+			$this->namespaceExaminer
 		);
 
-		$this->assertTrue(
-			$instance->modifyOutput( $output )
-		);
+		$template = new \stdClass;
+		$template->data = [];
+
+		$instance->modify( $output, $template );
 	}
 
 	public function testTryPrependHtmlOnNOBREADCRUMBLINKS() {
@@ -183,24 +202,28 @@ public function testTryPrependHtmlOnNOBREADCRUMBLINKS() {
 		$output->expects( $this->never() )
 			->method( 'prependHTML' );
 
-		$output->expects( $this->once() )
+		$output->expects( $this->any() )
 			->method( 'getContext' )
 			->will( $this->returnValue( $context ) );
 
 		$output->expects( $this->atLeastOnce() )
 			->method( 'getTitle' )
 			->will( $this->returnValue( $title ) );
 
-		$instance = new SkinTemplateOutputModifier( $htmlBreadcrumbLinksBuilder );
+		$instance = new SkinTemplateOutputModifier(
+			$htmlBreadcrumbLinksBuilder,
+			$this->namespaceExaminer
+		);
 
 		$output->smwmagicwords = [ 'SBL_NOBREADCRUMBLINKS' ];
 
-		$this->assertTrue(
-			$instance->modifyOutput( $output )
-		);
+		$template = new \stdClass;
+		$template->data = [];
+
+		$instance->modify( $output, $template );
 	}
 
-	public function testPrependHtmlForViewActionOnly() {
+	public function tesPrependHtmlForViewActionOnly() {
 
 		$context = new \RequestContext();
 		$context->setRequest( new \FauxRequest( [ 'action' => 'view'  ], true ) );
@@ -252,16 +275,15 @@ public function testPrependHtmlForViewActionOnly() {
 			->method( 'getTitle' )
 			->will( $this->returnValue( $title ) );
 
-		$instance = new SkinTemplateOutputModifier( $htmlBreadcrumbLinksBuilder );
-
-		$this->assertTrue(
-			$instance->modifyOutput( $output )
+		$instance = new SkinTemplateOutputModifier(
+			$htmlBreadcrumbLinksBuilder,
+			$this->namespaceExaminer
 		);
 
 		$template = new \stdClass;
 		$template->data = [];
 
-		$instance->modifyTemplate( $template );
+		$instance->modify( $output, $template );
 
 		$this->assertEmpty(
 			$template->data['subtitle']