NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[DomCrawler] prevent deprecation being triggered from assertion#35899

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:4.4fromxabbuh:issue-35889

Mar 2, 2020

Merged

[DomCrawler] prevent deprecation being triggered from assertion#35899

fabpot merged 1 commit intosymfony:4.4fromxabbuh:issue-35889

Mar 2, 2020

Conversation

Copy link

Member

xabbuh commentedFeb 28, 2020

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix#35889
License	MIT
Doc PR

xabbuh added this to the4.4 milestone

Feb 28, 2020

carsonbot added Status: Needs Review DomCrawler Bug labels

Feb 28, 2020

xabbuh mentioned this pull request

Feb 28, 2020

DOM Crawler component triggers deprecation error when calling its own method#35889

Closed

fancyweb approved these changes

Feb 28, 2020

View reviewed changes

Copy link

Contributor

fancyweb left a comment

Choose a reason for hiding this comment

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

As pointed in the issue, don't we need to also updatehttps://github.com/symfony/dom-crawler/blob/4.4/Test/Constraint/CrawlerSelectorTextSame.php#L48?

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Feb 28, 2020

prevent deprecation being triggered from assertion

b01a10c

xabbuh force-pushed theissue-35889 branch from08fd622 tob01a10cCompare

March 1, 2020 19:18

Copy link

MemberAuthor

xabbuh commentedMar 1, 2020

@fancyweb Indeed, I fixed it.

fabpot approved these changes

Mar 2, 2020

View reviewed changes

Copy link

Member

fabpot commentedMar 2, 2020

Thank you@xabbuh.

fabpot merged commit0936a4e intosymfony:4.4

Mar 2, 2020

xabbuh deleted the issue-35889 branch

March 2, 2020 12:27

This was referencedMar 27, 2020

Release v4.4.6#36232

Merged

Release v5.0.6#36240

Merged

dunglas mentioned this pull request

Mar 29, 2020

[DomCrawler] Fix BC break in assertions breaking Panther#36259

Merged

nicolas-grekas added a commit that referenced this pull request

Mar 30, 2020

bug#36259[DomCrawler] Fix BC break in assertions breaking Panther (…

ff2c362

…dunglas)This PR was merged into the 4.4 branch.Discussion----------[DomCrawler] Fix BC break in assertions breaking Panther| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | yes| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       | n/a| License       | MIT| Doc PR        | n/a#35899 introduces a BC break: browsers aren't able to retrieve the non-normalized version of a text. According to the HTML spec, whitespaces are always normalized. Because of this patch, these assertions doesn't work with Panther anymore.Also, this change probably hurts other users because getting the non-normalized version is almost never expected. (I'm in favor of **not** supporting retrieving the non-normalized version at all, for consistency with browsers and the spec, but it's another topic).Commits-------7af07c8 [DomCrawler] Fix BC break in assertions breaking Panther

iceuponfire mentioned this pull request

Mar 30, 2020

Panther only supports getting normalized textsymfony/panther#325

Closed

Copy link

flaushi commentedApr 2, 2020

Maybe we could add a hint toassertSelectorTextSame into the error message

symfony/src/Symfony/Component/DomCrawler/Crawler.php

Line 619 in15edfd3

@trigger_error(sprintf('"%s()" will normalize whitespaces by default in Symfony 5.0, set the second "$normalizeWhitespace" argument to false to retrieve the non-normalized version of the text.',__METHOD__),E_USER_DEPRECATED);

? I think many people had to research this issue

Copy link

MemberAuthor

xabbuh commentedApr 2, 2020

What issue would that solve?

Copy link

flaushi commentedApr 2, 2020

I can imagine may people using$crawler->text() in their tests.

They all have to read through this issue and PR to finally see that there is a new assertionassertSelectorTextSame they can use to prevent the deprecation error. Kind of "documentation in issue"