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 ( "" != "" ), 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 @@ {{alt}} + {{#caption}}
{{{caption}}}
{{/caption}} \ 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 @@ - \ No newline at end of file + 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 = 'default-alt'; + $string = ' + default-alt + default caption + '; $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 { ', 'description' => 'Only title' ], + [ + 'input' => [ + [ + 'type' => 'image', + 'data' => [ + 'alt' => 'image alt', + 'url' => 'http://image.jpg', + 'caption' => 'Lorem ipsum dolor' + ] + ] + ], + 'output' => '', + 'description' => 'Only image' + ], [ 'input' => [ [ @@ -75,27 +129,6 @@ class PortableInfoboxRenderServiceTest extends PHPUnit_Framework_TestCase { ', 'description' => 'Footer only' ], - [ - 'input' => [ - [ - 'type' => 'image', - 'data' => [ - 'alt' => 'image alt', - 'url' => 'http://image.jpg' - ] - ] - ], - 'output' => '', - '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' => '', + '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(''); + $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( ' + + + + + + + ' ); + + $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 */