Skip to content

Update Crawler.php #7963

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.

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 May 10, 2013
Merged

Update Crawler.php #7963

merged 1 commit into from May 10, 2013

Conversation

ghost
Copy link

@ghost ghost commented May 7, 2013

If HTML is not well-formed XML parsing goes wrong.
Could be useful create another method returning XML and XHTML in a well-formed standard?

If HTML is not well-formed XML parsing goes wrong.
@fabpot
Copy link
Member

fabpot commented May 7, 2013

As this is a new method in 2.3, I would rather add an argument to the html() method to be able to switch from HTML to XML. What do you think?

Perhaps that we should then find another name for the method itself (instead of html).

fabpot added a commit that referenced this pull request May 10, 2013
This PR was merged into the master branch.

Discussion
----------

Update Crawler.php

If HTML is not well-formed XML parsing goes wrong.
Could be useful create another method returning XML and XHTML in a `well-formed` standard?

Commits
-------

0a26641 Update Crawler.php
@fabpot fabpot closed this May 10, 2013
@fabpot fabpot merged commit 0a26641 into symfony:master May 10, 2013
@jakzal
Copy link
Contributor

jakzal commented May 10, 2013

@fabpot @spil-giacomo the node argument in the DOMDocument::saveHTML() was introduced in PHP 5.3.6.

In PHP 5.3.3 one of the tests fails:

1) Symfony\Component\DomCrawler\Tests\CrawlerTest::testHtml
DOMDocument::saveHTML() expects exactly 0 parameters, 1 given

/home/vagrant/Projects/symfony/src/Symfony/Component/DomCrawler/Crawler.php:502
/home/vagrant/Projects/symfony/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php:333

@ghost
Copy link
Author

ghost commented May 11, 2013

Yes, could be a solution now having one method to switch. I agree.
The only constraint could be if you start from a portions of HTML/XHTML/XML you can’t know what kind of document is.

fabpot added a commit that referenced this pull request May 13, 2013
This PR was merged into the master branch.

Discussion
----------

[DomCrawler] Fixed the Crawler::html() method for early PHP versions

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes, but not on travis (segfault)
| Fixed tickets | #7963
| License       | MIT
| Doc PR        | -

Node argument was added to the [`DOMDocument::saveHTML()`](http://php.net/manual/en/domdocument.savehtml.php) in PHP 5.3.6. See http://php.net/manual/en/domdocument.savehtml.php.

It's not a nice looking solution, but seems to be the only option. Condition should be removed once PHP dependency goes over 5.3.6.

Commits
-------

a4e3ebf [DomCrawler] Fixed the Crawler::html() method for PHP versions earlier than 5.3.6.
@fabpot fabpot mentioned this pull request Oct 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants