8000 Deprecate loading multiple documents in the same crawler by stof · Pull Request #16057 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Deprecate loading multiple documents in the same crawler #16057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 2, 2015
Merged

Deprecate loading multiple documents in the same crawler #16057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Deprecate loading multiple documents in the same crawler
  • Loading branch information
@stof
stof committed Oct 2, 2015
commit 0d1cb3bd1bc4f9726ad987cba8c2fa298187e9d0
15 changes: 15 additions & 0 deletions src/Symfony/Component/DomCrawler/Crawler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ class Crawler extends \SplObjectStorage
*/
private $baseHref;

/**
* @var \DOMDocument|null
*/
private $document;

/**
* Whether the Crawler contains HTML or XML content (used when converting CSS to XPath).
*
Expand Down Expand Up @@ -68,6 +73,7 @@ public function __construct($node = null, $currentUri = null, $baseHref = null)
public function clear()
{
parent::removeAll($this);
$this->document = null;
}

/**
Expand Down Expand Up @@ -307,6 +313,14 @@ public function addNodes(array $nodes)
*/
public function addNode(\DOMNode $node)
{
if (null !== $this->document && $this->document !== $node->ownerDocument) {
@trigger_error('Attaching DOM nodes from multiple documents in a Crawler is deprecated as of 2.8 and will be forbidden in 3.0.', E_USER_DEPRECATED);
}

if (null === $this->document) {
$this->document = $node->ownerDocument;
}

if ($node instanceof \DOMDocument) {
parent::attach($node->documentElement);
} else {
Expand Down Expand Up @@ -1152,6 +1166,7 @@ private function createSubCrawler($nodes)
{
$crawler = new static($nodes, $this->uri, $this->baseHref);
$crawler->isHtml = $this->isHtml;
$crawler->document = $this->document;

return $crawler;
}
Expand Down
19 changes: 17 additions & 2 deletions src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ public function testConstructor()
$crawler = new Crawler();
$this->assertCount(0, $crawler, '__construct() returns an empty crawler');

$crawler = new Crawler(new \DOMNode());
$doc = new \DOMDocument();
$node = $doc->createElement('test');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change should actually be done in older branches too. Passing an orphan DOMNode creates a broken crawler:

  • we actually only accept DOMElement (and DOMDocument where we get the documentElement in it), not any DOMNode (you get errors in some cases if you inject such nodes in a crawler)
  • we don't accept nodes not attached to a document (you would get an Invalid state error from the DOM library when accessing ownerDocument, which we already do when filtering the crawler)


$crawler = new Crawler($node);
$this->assertCount(1, $crawler, '__construct() takes a node as a first argument');
}

Expand Down Expand Up @@ -71,6 +74,14 @@ public function testAddHtmlContent()
$crawler->addHtmlContent('<html><div class="foo"></html>', 'UTF-8');

$this->assertEquals('foo', $crawler->filterXPath('//div')->attr('class'), '->addHtmlContent() adds nodes from an HTML string');
}

/**
* @covers Symfony\Component\DomCrawler\Crawler::addHtmlContent
*/
public function testAddHtmlContentWithBaseTag()
{
$crawler = new Crawler();

$crawler->addHtmlContent('<html><head><base href="http://symfony.com"></head><a href="/contact"></a></html>', 'UTF-8');

Expand Down Expand Up @@ -267,6 +278,7 @@ public function testAddNodeList()
*/
public function testAddNodes()
{
$list = array();
foreach ($this->createNodeList() as $node) {
$list[] = $node;
}
Expand All @@ -290,7 +302,10 @@ public function testAddNode()

public function testClear()
{
$crawler = new Crawler(new \DOMNode());
$doc = new \DOMDocument();
$node = $doc->createElement('test');

$crawler = new Crawler($node);
$crawler->clear();
$this->assertCount(0, $crawler, '->clear() removes all the nodes from the crawler');
}
Expand Down
0