From 01ccae3e79f12ae178cb49c970175f737143649d Mon Sep 17 00:00:00 2001
From: Diana <dianafa@o2.pl>
Date: Tue, 26 May 2015 10:51:41 +0000
Subject: [PATCH] merge with dev

---
 ...rtableInfoboxParserTagController.class.php |   2 +-
 services/Parser/Nodes/Node.php                |  27 ++++-
 services/Parser/Nodes/NodeComparison.php      |   2 +-
 services/Parser/Nodes/NodeData.php            |  11 ++
 services/Parser/Nodes/NodeGroup.php           |   2 +-
 services/Parser/Nodes/NodeImage.php           |   6 +-
 services/Parser/Nodes/NodeSet.php             |   2 +-
 services/Parser/XmlParser.php                 |  16 ++-
 .../PortableInfoboxRenderService.class.php    |  11 +-
 templates/PortableInfoboxItemImage.mustache   |   1 +
 templates/PortableInfoboxWrapper.mustache     |   2 +-
 tests/ParserNodesTest.php                     |  14 ++-
 tests/PortableInfoboxRenderServiceTest.php    | 100 ++++++++++++++----
 tests/XmlParserTest.php                       |  28 +++++
 14 files changed, 187 insertions(+), 37 deletions(-)

diff --git a/controllers/PortableInfoboxParserTagController.class.php b/controllers/PortableInfoboxParserTagController.class.php
index df84855..b7f5d9b 100644
--- a/controllers/PortableInfoboxParserTagController.class.php
+++ b/controllers/PortableInfoboxParserTagController.class.php
@@ -104,7 +104,7 @@ class PortableInfoboxParserTagController extends WikiaController {
 		$value = isset( $params[ 'theme-source' ] ) ? $frame->getArgument( $params[ 'theme-source' ] ) : false;
 		$themeName = $this->getThemeName( $params, $value );
 		//make sure no whitespaces, prevents side effects
-		return self::INFOBOX_THEME_PREFIX . preg_replace( '|\s+|s', '-', $themeName );
+		return Sanitizer::escapeClass( self::INFOBOX_THEME_PREFIX . preg_replace( '|\s+|s', '-', $themeName ) );
 	}
 
 	private function getThemeName( $params, $value ) {
diff --git a/services/Parser/Nodes/Node.php b/services/Parser/Nodes/Node.php
index 145d092..aa5efdf 100644
--- a/services/Parser/Nodes/Node.php
+++ b/services/Parser/Nodes/Node.php
@@ -12,6 +12,7 @@ class Node {
 	const LABEL_TAG_NAME = 'label';
 
 	protected $xmlNode;
+	protected $parent = null;
 
 	/* @var $externalParser ExternalParser */
 	protected $externalParser;
@@ -21,6 +22,24 @@ class Node {
 		$this->infoboxData = $infoboxData;
 	}
 
+	/**
+	 * @return mixed
+	 */
+	public function getParent() {
+		return $this->parent;
+	}
+
+	/**
+	 * @param mixed $parent
+	 */
+	public function setParent( Node $parent ) {
+		$this->parent = $parent;
+	}
+
+	public function ignoreNodeWhenEmpty() {
+		return true;
+	}
+
 	/**
 	 * @return ExternalParser
 	 */
@@ -39,7 +58,13 @@ class Node {
 	}
 
 	public function getType() {
-		return $this->xmlNode->getName();
+		/*
+		 * Node type generation is based on XML tag name.
+		 * It's worth to remember that SimpleXMLElement::getName method is
+		 * case - sensitive ( "<Data>" != "<data>" ), so we need to sanitize Node Type
+		 * by using strtolower function
+		 */
+		return strtolower( $this->xmlNode->getName() );
 	}
 
 	public function getData() {
diff --git a/services/Parser/Nodes/NodeComparison.php b/services/Parser/Nodes/NodeComparison.php
index 05fc46c..5ab0058 100644
--- a/services/Parser/Nodes/NodeComparison.php
+++ b/services/Parser/Nodes/NodeComparison.php
@@ -10,7 +10,7 @@ class NodeComparison extends Node {
 		if ( $this->externalParser ) {
 			$nodeFactory->setExternalParser( $this->externalParser );
 		}
-		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ];
+		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode, $this ) ];
 	}
 
 	public function isEmpty( $data ) {
diff --git a/services/Parser/Nodes/NodeData.php b/services/Parser/Nodes/NodeData.php
index a529f48..76392a4 100644
--- a/services/Parser/Nodes/NodeData.php
+++ b/services/Parser/Nodes/NodeData.php
@@ -3,6 +3,17 @@ namespace Wikia\PortableInfobox\Parser\Nodes;
 
 class NodeData extends Node {
 
+	public function ignoreNodeWhenEmpty() {
+		$parent = $this->getParent();
+		if ( $parent instanceof NodeSet ) {
+			if ( $parent->getParent() instanceof NodeComparison ) {
+				// data tag inside comparison tag can not be ignored
+				return false;
+			}
+		}
+		return true;
+	}
+
 	public function getData() {
 		return [
 			'label' => $this->getExternalParser()->parseRecursive(
diff --git a/services/Parser/Nodes/NodeGroup.php b/services/Parser/Nodes/NodeGroup.php
index 49d4af2..0e266bf 100644
--- a/services/Parser/Nodes/NodeGroup.php
+++ b/services/Parser/Nodes/NodeGroup.php
@@ -10,7 +10,7 @@ class NodeGroup extends Node {
 		if ( $this->externalParser ) {
 			$nodeFactory->setExternalParser( $this->externalParser );
 		}
-		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ];
+		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode, $this ) ];
 	}
 
 	public function isEmpty( $data ) {
diff --git a/services/Parser/Nodes/NodeImage.php b/services/Parser/Nodes/NodeImage.php
index 4206707..42cb31e 100644
--- a/services/Parser/Nodes/NodeImage.php
+++ b/services/Parser/Nodes/NodeImage.php
@@ -5,18 +5,22 @@ use Wikia\PortableInfobox\Helpers\ImageFilenameSanitizer;
 
 class NodeImage extends Node {
 	const ALT_TAG_NAME = 'alt';
+	const CAPTION_TAG_NAME = 'caption';
 
 	public function getData() {
 		$title = $this->getImageAsTitleObject( $this->getRawValueWithDefault( $this->xmlNode ) );
 		$ref = null;
 		$alt = $this->getValueWithDefault( $this->xmlNode->{self::ALT_TAG_NAME} );
+		$caption = $this->getValueWithDefault( $this->xmlNode->{self::CAPTION_TAG_NAME} );
 
 		wfRunHooks( 'PortableInfoboxNodeImage::getData', [ $title, &$ref, $alt ] );
 
 		return [
 			'url' => $this->resolveImageUrl( $title ),
-			'name' => ( $title ) ? $title->getDBkey() : '',
+			'name' => ( $title ) ? $title->getText() : '',
+			'key' => ( $title ) ? $title->getDBKey() : '',
 			'alt' => $alt,
+			'caption' => $caption,
 			'ref' => $ref
 		];
 	}
diff --git a/services/Parser/Nodes/NodeSet.php b/services/Parser/Nodes/NodeSet.php
index e725052..49bb629 100644
--- a/services/Parser/Nodes/NodeSet.php
+++ b/services/Parser/Nodes/NodeSet.php
@@ -10,7 +10,7 @@ class NodeSet extends Node {
 		if ( $this->externalParser ) {
 			$nodeFactory->setExternalParser( $this->externalParser );
 		}
-		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ];
+		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode, $this ) ];
 	}
 
 	public function isEmpty( $data ) {
diff --git a/services/Parser/XmlParser.php b/services/Parser/XmlParser.php
index d34cd3c..0ea5416 100644
--- a/services/Parser/XmlParser.php
+++ b/services/Parser/XmlParser.php
@@ -28,16 +28,18 @@ class XmlParser {
 	 * @param \SimpleXMLElement $xmlIterable
 	 * @return array
 	 */
-	public function getDataFromNodes( \SimpleXMLElement $xmlIterable ) {
+	public function getDataFromNodes( \SimpleXMLElement $xmlIterable, $parentNode = null ) {
 		wfProfileIn(__METHOD__);
 		$data = [ ];
 		foreach ( $xmlIterable as $node ) {
-			$nodeHandler = $this->getNode( $node );
+			$nodeHandler = $this->getNode( $node, $parentNode );
 			$nodeData = $nodeHandler->getData();
-			if ( !$nodeHandler->isEmpty( $nodeData ) ) {
+			// add data if node is not empty or - when node can not be ignored when empty
+			if ( !$nodeHandler->isEmpty( $nodeData ) || !$nodeHandler->ignoreNodeWhenEmpty() ) {
 				$data[ ] = [
 					'type' => $nodeHandler->getType(),
-					'data' => $nodeData
+					'data' => $nodeData,
+					'isEmpty' => $nodeHandler->isEmpty( $nodeData )
 				];
 			}
 		}
@@ -81,9 +83,10 @@ class XmlParser {
 
 	/**
 	 * @param \SimpleXMLElement $xmlNode
+	 * @param Node $parent
 	 * @return \Wikia\PortableInfobox\Parser\Nodes\Node
 	 */
-	public function getNode( \SimpleXMLElement $xmlNode ) {
+	public function getNode( \SimpleXMLElement $xmlNode, $parent = null ) {
 		wfProfileIn(__METHOD__);
 		$tagType = $xmlNode->getName();
 		$className = 'Wikia\\PortableInfobox\\Parser\\Nodes\\' . 'Node' . ucfirst( strtolower( $tagType ) );
@@ -93,6 +96,9 @@ class XmlParser {
 			if ( !empty( $this->externalParser ) ) {
 				$instance->setExternalParser( $this->externalParser );
 			}
+			if ( $parent ) {
+				$instance->setParent( $parent );
+			}
 			wfProfileOut( __METHOD__ );
 			return $instance;
 		}
diff --git a/services/PortableInfoboxRenderService.class.php b/services/PortableInfoboxRenderService.class.php
index c31c2af..388bee2 100644
--- a/services/PortableInfoboxRenderService.class.php
+++ b/services/PortableInfoboxRenderService.class.php
@@ -150,8 +150,9 @@ class PortableInfoboxRenderService extends WikiaService {
 		// it modular) While doing this we also need to move this logic to appropriate image render class
 		if ( $type === 'image' ) {
 			$data[ 'thumbnail' ] = $this->getThumbnailUrl( $data );
+			$data[ 'key' ] = urlencode( $data[ 'key' ] );
 
-			if ( F::app()->checkSkin( 'wikiamobile' ) ) {
+			if ( $this->isWikiaMobile() ) {
 				$type = $type . self::MOBILE_TEMPLATE_POSTFIX;
 			}
 		}
@@ -185,7 +186,7 @@ class PortableInfoboxRenderService extends WikiaService {
 	private function createVignetteThumbnail( $url ) {
 		return VignetteRequest::fromUrl( $url )
 			->thumbnailDown()
-			->width( F::app()->checkSkin( 'wikiamobile' ) ?
+			->width( $this->isWikiaMobile() ?
 				self::MOBILE_THUMBNAIL_WIDTH :
 				self::DESKTOP_THUMBNAIL_WIDTH
 			)
@@ -211,6 +212,12 @@ class PortableInfoboxRenderService extends WikiaService {
 			);
 		}
 		return '';
+	/**
+	 * required for testing mobile template rendering
+	 * @return bool
+	 */
+	protected function isWikiaMobile() {
+		return F::app()->checkSkin( 'wikiamobile' );
 	}
 
 	/**
diff --git a/templates/PortableInfoboxItemImage.mustache b/templates/PortableInfoboxItemImage.mustache
index cbf0714..400ea65 100644
--- a/templates/PortableInfoboxItemImage.mustache
+++ b/templates/PortableInfoboxItemImage.mustache
@@ -3,5 +3,6 @@
 		<a href="{{url}}" class="image image-thumbnail" title="{{alt}}">
 			<img src="{{thumbnail}}" class="portable-infobox-image" alt="{{alt}}" data-image-key="{{name}}" data-image-name="{{name}}"/>
 		</a>
+		{{#caption}}<figcaption class="portable-infobox-item-margins portable-infobox-image-caption">{{{caption}}}</figcaption>{{/caption}}
 	</figure>
 </div>
\ No newline at end of file
diff --git a/templates/PortableInfoboxWrapper.mustache b/templates/PortableInfoboxWrapper.mustache
index 4fb4e57..b13bc38 100644
--- a/templates/PortableInfoboxWrapper.mustache
+++ b/templates/PortableInfoboxWrapper.mustache
@@ -1 +1 @@
-<aside class="portable-infobox{{#theme}} {{{theme}}}{{/theme}}">{{{content}}}</aside>
\ No newline at end of file
+<aside class="portable-infobox{{#theme}} {{theme}}{{/theme}}">{{{content}}}</aside>
diff --git a/tests/ParserNodesTest.php b/tests/ParserNodesTest.php
index 72ede5b..00ee644 100644
--- a/tests/ParserNodesTest.php
+++ b/tests/ParserNodesTest.php
@@ -28,17 +28,27 @@ class PortableInfoboxParserNodesTest extends WikiaBaseTest {
 	}
 
 	public function testNodeImage() {
-		$string = '<image source="image2"><alt source="alt-source"><default>default-alt</default></alt></image>';
+		$string = '<image source="image2">
+						<alt source="alt-source"><default>default-alt</default></alt>
+						<caption source="caption"><default>default caption</default></caption>
+					</image>';
 		$xml = simplexml_load_string( $string );
 
 		$nodeDefault = new Wikia\PortableInfobox\Parser\Nodes\NodeImage( $xml, [ ] );
 
-		$node = $this->getMockBuilder( 'Wikia\PortableInfobox\Parser\Nodes\NodeImage' )->setConstructorArgs( [ $xml, [ 'image2' => 'aaa.jpg', 'alt-source' => 'bbb' ] ] )->setMethods( [ 'resolveImageUrl' ] )->getMock();
+		$node = $this->getMockBuilder( 'Wikia\PortableInfobox\Parser\Nodes\NodeImage' )
+						->setConstructorArgs( [ $xml, [ 'image2' => 'aaa.jpg',
+														'alt-source' => 'bbb',
+														'caption' => 'capt' ] ] )
+						->setMethods( [ 'resolveImageUrl' ] )
+						->getMock();
 		$node->expects( $this->any() )->method( 'resolveImageUrl' )->will( $this->returnValue( 'aaa.jpg' ) );
 		$this->assertTrue( $node->getData()[ 'url' ] == 'aaa.jpg', 'value is not aaa.jpg' );
 		$this->assertTrue( $node->getData()[ 'name' ] == 'Aaa.jpg', 'value is not aaa.jpg' );
 		$this->assertTrue( $node->getData()[ 'alt' ] == 'bbb', 'alt is not bbb' );
+		$this->assertTrue( $node->getData()[ 'caption' ] == 'capt', 'caption is not "capt"' );
 		$this->assertTrue( $nodeDefault->getData()[ 'alt' ] == 'default-alt', 'default alt' );
+		$this->assertTrue( $nodeDefault->getData()[ 'caption' ] == 'default caption', 'default caption' );
 	}
 
 	public function testNodeHeader() {
diff --git a/tests/PortableInfoboxRenderServiceTest.php b/tests/PortableInfoboxRenderServiceTest.php
index 4e8116e..88f2865 100644
--- a/tests/PortableInfoboxRenderServiceTest.php
+++ b/tests/PortableInfoboxRenderServiceTest.php
@@ -13,6 +13,11 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase {
 		$this->infoboxRenderService = $mock;
 	}
 
+	private function setWikiaMobileSkin($bool) {
+		$this->infoboxRenderService->expects( $this->any() )->method( 'isWikiaMobile' )->will( $this->returnValue
+		($bool) );
+	}
+
 	/**
 	 * @param $input
 	 * @param $expectedOutput
@@ -20,6 +25,7 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase {
 	 * @dataProvider testRenderInfoboxDataProvider
 	 */
 	public function testRenderInfobox( $input, $expectedOutput, $description ) {
+		$this->setWikiaMobileSkin(false);
 
 		$actualOutput = $this->infoboxRenderService->renderInfobox( $input );
 
@@ -38,6 +44,31 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase {
 		$this->assertEquals( $expectedHtml, $actualHtml, $description );
 	}
 
+	/**
+	 * @param $input
+	 * @param $expectedOutput
+	 * @param $description
+	 * @dataProvider testRenderInfoboxDataProvider
+	 */
+	public function testRenderMobileInfobox( $input, $expectedOutput, $description ) {
+		$this->setWikiaMobileSkin(true);
+		$actualOutput = $this->infoboxRenderService->renderInfobox( $input );
+
+		$actualDOM = new DOMDocument('1.0');
+		$expectedDOM = new DOMDocument('1.0');
+		$actualDOM->formatOutput = true;
+		$actualDOM->preserveWhiteSpace = false;
+		$expectedDOM->formatOutput = true;
+		$expectedDOM->preserveWhiteSpace = false;
+		$actualDOM->loadXML($actualOutput);
+		$expectedDOM->loadXML($expectedOutput);
+
+		$expectedHtml = $expectedDOM->saveXML();
+		$actualHtml = $actualDOM->saveXML();
+
+		$this->assertEquals( $expectedHtml, $actualHtml, $description );
+	}
+
 	public function testRenderInfoboxDataProvider() {
 		return [
 			[
@@ -61,6 +92,29 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase {
 							</aside>',
 				'description' => 'Only title'
 			],
+			[
+				'input' => [
+					[
+						'type' => 'image',
+						'data' => [
+							'alt' => 'image alt',
+							'url' => 'http://image.jpg',
+							'caption' => 'Lorem ipsum dolor'
+						]
+					]
+				],
+				'output' => '<aside class="portable-infobox">
+								<div class="portable-infobox-item item-type-image no-margins">
+									<figure class="portable-infobox-image-wrapper">
+										<a href="http://image.jpg" class="image image-thumbnail" title="image alt">
+											<img src="http://image.jpg" class="portable-infobox-image" alt="image alt" data-image-key="" data-image-name=""/>
+										</a>
+										<figcaption class="portable-infobox-item-margins portable-infobox-image-caption">Lorem ipsum dolor</figcaption>
+									</figure>
+								</div>
+							</aside>',
+				'description' => 'Only image'
+			],
 			[
 				'input' => [
 					[
@@ -75,27 +129,6 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase {
 							</aside>',
 				'description' => 'Footer only'
 			],
-			[
-				'input' => [
-					[
-						'type' => 'image',
-						'data' => [
-							'alt' => 'image alt',
-							'url' => 'http://image.jpg'
-						]
-					]
-				],
-				'output' => '<aside class="portable-infobox">
-								<div class="portable-infobox-item item-type-image no-margins">
-									<figure class="portable-infobox-image-wrapper">
-										<a href="http://image.jpg" class="image image-thumbnail" title="image alt">
-											<img src="http://image.jpg" class="portable-infobox-image" alt="image alt" data-image-key="" data-image-name=""/>
-										</a>
-									</figure>
-								</div>
-							</aside>',
-				'description' => 'Only image'
-			],
 			[
 				'input' => [
 					[
@@ -322,4 +355,29 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase {
 			]
 		];
 	}
+
+	public function testRenderMobileInfoboxDataProvider() {
+		return [
+			[
+				'input' => [
+					[
+						'type' => 'image',
+						'data' => [
+							'alt' => 'image alt',
+							'url' => 'http://image.jpg',
+							'thumbnail' => 'thumbnail.jpg',
+							'ref' => 1,
+							'name' => 'test1'
+						]
+					]
+				],
+				'output' => '<aside class="portable-infobox">
+								<div class="portable-infobox-item item-type-image no-margins">
+									<img src="data:image/gif;base64,R0lGODlhAQABAIABAAAAAP///yH5BAEAAAEALAAAAAABAAEAQAICTAEAOw%3D%3D" class="portable-infobox-image lazy media article-media" alt="image alt"  data-image-key="test1" data-image-name="test1" data-ref="1" data-src="thumbnail.jpg" data-params=\'[{"name":"test1", "full":"http://image.jpg"}]\' />
+								</div>
+							</aside>',
+				'description' => 'Only image for mobile'
+			]
+		];
+	}
 }
diff --git a/tests/XmlParserTest.php b/tests/XmlParserTest.php
index 48e6676..ea725da 100644
--- a/tests/XmlParserTest.php
+++ b/tests/XmlParserTest.php
@@ -59,6 +59,34 @@ class XmlParserTest extends WikiaBaseTest {
 		$this->assertTrue( $data[1]['data']['value'][0]['data']['value'][2]['data']['value'] == 'parseRecursive(LALALA)' );
 	}
 
+	public function testNotProvidingDataSource() {
+		$xmlParser = new \Wikia\PortableInfobox\Parser\XmlParser( ['a'=>'1'] );
+
+		$data1 = $xmlParser->getDataFromXmlString('<infobox><data source="b" /><data source="a" /></infobox>');
+		$this->assertTrue( $data1[0]['data']['value'] == '1', 'Data with empty source "b" should be ignored' );
+	}
+
+	public function testNotProvidingDataSourceInsideComparison() {
+		$xmlParser = new \Wikia\PortableInfobox\Parser\XmlParser( ['a'=>'1'] );
+
+		$data1 = $xmlParser->getDataFromXmlString( '<infobox>
+														<comparison>
+															<set>
+																<data source="b" />
+																<data source="a" />
+															</set>
+														</comparison>
+													 </infobox>' );
+
+		$setElement = $data1[0]['data']['value'][0]['data']['value'];
+
+		//Data with empty source "b" should not be ignored inside comparison:
+		$this->assertTrue( $setElement[0]['data']['value'] == '', 'Data with empty source "b" inside comparison' );
+		$this->assertTrue( $setElement[0]['isEmpty'] == true );
+
+		$this->assertTrue( $setElement[1]['data']['value'] == '1', '' );
+	}
+
 	/**
 	 * @dataProvider errorHandlingDataProvider
 	 */