merged branch matthijsvandenbos/matthijsvandenbos/basetag-href-fix (PR #7178)

fabpot · fabpot · commit 0c09b9392a52 · 2013-02-26T17:49:54.000+01:00
This PR was submitted for the master branch but it was merged into the 2.1 branch instead (closes #7178). Commits ------- c41f640 [DomCrawler] Fixed handling absent href attribute in base tag Discussion ---------- [DomCrawler] Fixed handling absent href attribute in base tag | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | # Description The HTML5 spec states that the href attribute is optional for the base tag. The current code causes an exception on conforming HTML. Fixed the DomCrawler::addHtmlContent() method to support this. Added Unit Test to check for this situation. # Explanantion Currently, if the base tag doesn't have an href attribute, the uri for the DomCrawler gets set to an empty string. This is incorrect behaviour, especially because it breaks DomCrawler::links(). The Symfony\Component\DomCrawler\Link objects it creates, expect a non-empty string in their constructor arguments and throw an InvalidArgumentException. # References http://www.w3.org/TR/html-markup/base.html#base.attrs.href http://www.whatwg.org/specs/web-apps/current-work/multipage/semantics.html#the-base-element
diff --git a/src/Symfony/Component/DomCrawler/Crawler.php b/src/Symfony/Component/DomCrawler/Crawler.php
@@ -145,8 +145,9 @@ public function addHtmlContent($content, $charset = 'UTF-8')
 
         $base = $this->filterXPath('descendant-or-self::base')->extract(array('href'));
 
-        if (count($base)) {
-            $this->uri = current($base);
+        $baseHref = current($base);
+        if (count($base) && !empty($baseHref)) {
+            $this->uri = $baseHref;
         }
     }
 
diff --git a/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php b/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php
@@ -80,6 +80,18 @@ public function testAddHtmlContentCharset()
         $this->assertEquals('Tiếng Việt', $crawler->filterXPath('//div')->text());
     }
 
+    /**
+     * @covers Symfony\Component\DomCrawler\Crawler::addHtmlContent
+     */
+    public function testAddHtmlContentInvalidBaseTag()
+    {
+        $crawler = new Crawler(null, 'http://symfony.com');
+
+        $crawler->addHtmlContent('<html><head><base target="_top"></head><a href="/contact"></a></html>', 'UTF-8');
+
+        $this->assertEquals('http://symfony.com/contact', current($crawler->filterXPath('//a')->links())->getUri(), '->addHtmlContent() correctly handles a non-existent base tag href attribute');
+    }
+
     /**
      * @covers Symfony\Component\DomCrawler\Crawler::addHtmlContent
      */

Original file line number	Diff line number	Diff line change
`@@ -145,8 +145,9 @@ public function addHtmlContent($content, $charset = 'UTF-8')`
`145`	`145`
`146`	`146`	`$base = $this->filterXPath('descendant-or-self::base')->extract(array('href'));`
`147`	`147`
`148`		`- if (count($base)) {`
`149`		`- $this->uri = current($base);`
	`148`	`+ $baseHref = current($base);`
	`149`	`+ if (count($base) && !empty($baseHref)) {`
	`150`	`+ $this->uri = $baseHref;`
`150`	`151`	`}`
`151`	`152`	`}`
`152`	`153`