Merge pull request #18 from andreskrey/development

Prepare for release v0.2.0

Author: andreskrey

.travis.yml

@@ -7,6 +7,6 @@ php:
   - "5.5"
   - "5.6"
   - "7.0"
-  - "hhvm"
+  - "7.1"
 
 sudo: false

CHANGELOG.md

@@ -3,6 +3,25 @@ All notable changes to this project will be documented in this file.
 
 ## Unreleased
 
+## [v0.2.0](https://github.com/andreskrey/readability.php/releases/tag/v0.2.0)
+
+We ARE a 100% complete por of Readability.js!
+- Every test unit passes
+- Readability.php produces the same exact output as Readability.js
+- I'm happy :)
+
+### Fixed
+- Lots of bugs
+- Merged PR by DavidFricker to avoid exceptions while grabbing the document content
+
+### Added
+- substituteEntities flag, to avoid replacing especial characters with HTML entities. There's nothing we can do about ` `, that entity is replaced by libxml and there's no way to disable it.
+- Named data sets so it's easier to detect which test case is failing.
+
+### Removed
+
+- Couple of test cases that involved broken JS. There's nothing we can do about JS spilling onto the text.
+
 ## [0.0.3-alpha](https://github.com/andreskrey/readability.php/releases/tag/v0.0.3v-alpha)
 
 We are getting closer to be a 100% complete port of Readability.js!

README.md

@@ -51,7 +51,8 @@ If the parsing process was unsuccessful the HTMLParser will return `false`
 - **weightClasses**: default value `true`, weight classes during the rating phase. 
 - **removeReadabilityTags**: default value `true`, remove the data-readability tags inside the nodes that are added during the rating phase. 
 - **fixRelativeURLs**: default value `false`, convert relative URLs to absolute. Like `/test` to `http://host/test`. 
-- **originalURL**: default value `http://fakehost`, original URL from the article used to fix relative URLs. 
+- **substituteEntities**: default value `false`, disables the `substituteEntities` flag of libxml. Will avoid substituting HTML entities. Like `´` to á.
+- **originalURL**: default value `http://fakehost`, original URL from the article used to fix relative URLs.
 
 ## Limitations
 

src/HTMLParser.php

@@ -26,11 +26,6 @@ class HTMLParser
      */
     private $metadata = [];
 
-    /**
-     * @var array
-     */
-    private $title = [];
-
     /**
      * @var array
      */
@@ -40,7 +35,7 @@ class HTMLParser
         'extraneous' => '/print|archive|comment|discuss|e[\-]?mail|share|reply|all|login|sign|single|utility/i',
         'byline' => '/byline|author|dateline|writtenby|p-author/i',
         'replaceFonts' => '/<(\/?)font[^>]*>/gi',
-        'normalize' => '/\s{2,}/g',
+        'normalize' => '/\s{2,}/',
         'videos' => '/\/\/(www\.)?(dailymotion|youtube|youtube-nocookie|player\.vimeo)\.com/i',
         'nextLink' => '/(next|weiter|continue|>([^\|]|$)|»([^\|]|$))/i',
         'prevLink' => '/(prev|earl|old|new|<|«)/i',
@@ -104,6 +99,7 @@ public function __construct(array $options = [])
             'weightClasses' => true,
             'removeReadabilityTags' => true,
             'fixRelativeURLs' => false,
+            'substituteEntities' => true,
             'originalURL' => 'http://fakehost',
         ];
 
@@ -137,7 +133,9 @@ public function parse($html)
 
         $this->metadata = $this->getMetadata();
 
-        $this->title = $this->getTitle();
+        $this->metadata['image'] = $this->getMainImage();
+
+        $this->metadata['title'] = $this->getTitle();
 
         // Checking for minimum HTML to work with.
         if (!($root = $this->dom->getElementsByTagName('body')->item(0))) {
@@ -162,7 +160,11 @@ public function parse($html)
 
             // TODO Better way to count resulting text. Textcontent usually has alt titles and that stuff
             // that doesn't really count to the quality of the result.
-            if ($result && mb_strlen($result->textContent) < 500) {
+            $length = 0;
+            foreach ($result->getElementsByTagName('p') as $p) {
+                $length += mb_strlen($p->textContent);
+            }
+            if ($result && mb_strlen(preg_replace('/\s/', '', $result->textContent)) < 500) {
                 $root = $this->backupdom->getElementsByTagName('body')->item(0);
 
                 if ($this->getConfig()->getOption('stripUnlikelyCandidates')) {
@@ -205,6 +207,11 @@ public function parse($html)
      */
     private function loadHTML($html)
     {
+        if (!$this->getConfig()->getOption('substituteEntities')) {
+            // Keep the original HTML entities
+            $this->dom->substituteEntities = false;
+        }
+
         // Prepend the XML tag to avoid having issues with special characters. Should be harmless.
         $this->dom->loadHTML('<?xml encoding="UTF-8">' . $html);
         $this->dom->encoding = 'UTF-8';
@@ -293,6 +300,15 @@ private function prepDocument()
                 }
             }
         }
+
+        // Replace font tags with span
+        $fonts = $this->dom->getElementsByTagName('font');
+        $length = $fonts->length;
+        for ($i = 0; $i < $length; $i++) {
+            $font = $fonts->item($length - 1 - $i);
+            $span = new Readability($font);
+            $span->setNodeTag('span', true);
+        }
     }
 
     public function postProcessContent(DOMDocument $article)
@@ -436,11 +452,39 @@ private function getMetadata()
 
         if (array_key_exists('og:image', $values) || array_key_exists('twitter:image', $values)) {
             $metadata['image'] = ($values['og:image']) ? $values['og:image'] : $values['twitter:image'];
+        } else {
+            $metadata['image'] = null;
         }
 
         return $metadata;
     }
 
+    /**
+     * Tries to get the main article image. Will only update the metadata if the getMetadata function couldn't
+     * find a correct image.
+     *
+     * @return bool|string URL of the top image or false if unsuccessful.
+     */
+    public function getMainImage()
+    {
+        if ($this->metadata['image'] !== null) {
+            return $this->metadata['image'];
+        }
+
+        foreach ($this->dom->getElementsByTagName('link') as $link) {
+            /** @var \DOMElement $link */
+            /*
+             * Check for the rel attribute, then check if the rel attribute is either img_src or image_src, and
+             * finally check for the existence of the href attribute, which should hold the image url.
+             */
+            if ($link->hasAttribute('rel') && ($link->getAttribute('rel') === 'img_src' || $link->getAttribute('rel') === 'image_src') && $link->hasAttribute('href')) {
+                return $link->getAttribute('href');
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Get the density of links as a percentage of the content
      * This is the amount of text that is inside a link divided by the total text in the node.
@@ -493,6 +537,7 @@ private function getTitle()
      * Gets nodes from the root element.
      *
      * @param $node Readability
+     *
      * @return array
      */
     private function getNodes(Readability $node)
@@ -586,7 +631,7 @@ private function rateNodes($nodes)
                 continue;
             }
             // Discard nodes with less than 25 characters, without blank space
-            if (mb_strlen($node->getValue(true)) < 25) {
+            if (mb_strlen($node->getTextContent(true)) < 25) {
                 continue;
             }
 
@@ -601,10 +646,10 @@ private function rateNodes($nodes)
             $contentScore = 1;
 
             // Add points for any commas within this paragraph.
-            $contentScore += count(explode(',', $node->getValue(true)));
+            $contentScore += count(explode(',', $node->getTextContent(true)));
 
             // For every 100 characters in this paragraph, add another point. Up to 3 points.
-            $contentScore += min(floor(mb_strlen($node->getValue(true)) / 100), 3);
+            $contentScore += min(floor(mb_strlen($node->getTextContent(true)) / 100), 3);
 
             // Initialize and score ancestors.
             /** @var Readability $ancestor */
@@ -1066,13 +1111,20 @@ public function removeNode(\DOMNode $node)
      * Checks if the node is a byline.
      *
      * @param Readability $node
-     * @param string $matchString
+     * @param string      $matchString
      *
      * @return bool
      */
     private function checkByline($node, $matchString)
     {
-        if ($this->getConfig()->getOption('articleByLine')) {
+        if (!$this->getConfig()->getOption('articleByLine')) {
+            return false;
+        }
+
+        /*
+         * Check if the byline is already set
+         */
+        if (isset($this->metadata['byline'])) {
             return false;
         }
 

src/Readability.php

@@ -286,8 +286,9 @@ public function getTextContent($normalize = false)
      * element with the new tag name and importing it to the main DOMDocument.
      *
      * @param string $value
+     * @param bool   $importAttributes
      */
-    public function setNodeTag($value)
+    public function setNodeTag($value, $importAttributes = false)
     {
         $new = new \DOMDocument();
         $new->appendChild($new->createElement($value));
@@ -298,6 +299,13 @@ public function setNodeTag($value)
             $new->firstChild->appendChild($import);
         }
 
+        if ($importAttributes) {
+            // Import attributes from the original node.
+            foreach ($this->node->attributes as $attribute) {
+                $new->firstChild->setAttribute($attribute->nodeName, $attribute->nodeValue);
+            }
+        }
+
         // The import must be done on the firstChild of $new, since $new is a DOMDocument and not a DOMElement.
         $import = $this->node->ownerDocument->importNode($new->firstChild, true);
         $this->node->parentNode->replaceChild($import, $this->node);

test/HTMLParserTest.php

@@ -9,11 +9,18 @@ class HTMLParserTest extends \PHPUnit_Framework_TestCase
     /**
      * @dataProvider getSamplePages
      */
-    public function testHTMLParserParsesHTML($html, $expectedResult, $expectedMetadata)
+    public function testHTMLParserParsesHTML($html, $expectedResult, $expectedMetadata, $config)
     {
-        $readability = new HTMLParser([
-            'originalURL' => 'http://fakehost/test/test.html',
-        ]);
+        $options = ['originalURL' => 'http://fakehost/test/test.html',
+            'fixRelativeURLs' => true,
+            'substituteEntities' => true,
+        ];
+
+        if ($config) {
+            $options = $config;
+        }
+
+        $readability = new HTMLParser($options);
         $result = $readability->parse($html);
 
         $this->assertEquals($expectedResult, $result['html']);
@@ -33,8 +40,12 @@ public function getSamplePages()
             $source = file_get_contents($path . DIRECTORY_SEPARATOR . $testPage . DIRECTORY_SEPARATOR . 'source.html');
             $expectedHTML = file_get_contents($path . DIRECTORY_SEPARATOR . $testPage . DIRECTORY_SEPARATOR . 'expected.html');
             $expectedMetadata = file_get_contents($path . DIRECTORY_SEPARATOR . $testPage . DIRECTORY_SEPARATOR . 'expected-metadata.json');
+            $config = file_get_contents($path . DIRECTORY_SEPARATOR . $testPage . DIRECTORY_SEPARATOR . 'config.json');
+            if ($config) {
+                $config = json_decode($config, true);
+            }
 
-            $pages[] = [$source, $expectedHTML, $expectedMetadata];
+            $pages[$testPage] = [$source, $expectedHTML, $expectedMetadata, $config];
         }
 
         return $pages;

test/ReadabilityTest.php

@@ -4,4 +4,9 @@
 
 class ReadabilityTest extends \PHPUnit_Framework_TestCase
 {
+    public function testDummy()
+    {
+        //TODO
+        $this->assertEquals(true, true);
+    }
 }

test/test-pages/ars-1/config.json

@@ -0,0 +1,3 @@
+{
+  "articleByLine": true
+}

test/test-pages/clean-links/expected.html

@@ -1,6 +1,6 @@
 <div><td>
         <h3 align="center ">Study Webtext</h3>
-        <h2 align="center "><font color="Maroon&#xA;        " face="Lucida Handwriting ">"Bartleby the Scrivener: A Story of Wall-Street " </font>(1853) <br></br>
+        <h2 align="center "><span color="Maroon&#xA;        " face="Lucida Handwriting ">"Bartleby the Scrivener: A Story of Wall-Street " </span>(1853) <br></br>
           Herman Melville</h2>
         <h2 align="center "><a href="http://www.vcu.edu/engweb/webtexts/bartleby.html&#xA;        " target="_blank "><img align="absmiddle " alt="To the story text without notes&#xA;        " height="38 " src="http://fakehost/test/hmhome.gif " width="38 "></img></a> 
         </h2>