[DomCrawler] normalizeWhitespace should be true by default #34151
Conversation
| @@ -614,6 +614,14 @@ public function text(/* string $default = null, bool $normalizeWhitespace = fals | |||
|
|
|||
| $text = $this->getNode(0)->nodeValue; | |||
|
|
|||
| if (\func_num_args() <= 1) { | |||
| if (trim(preg_replace('/(?:\s{2,}+|[^\S ])/', ' ', $text)) !== $text) { | |||
| @trigger_error(sprintf('"%s()" will normalize whitespaces by default in Symfony 5.0, set the 2nd "$normalizeWhitespace" argument to false to retrieve the non-normalized version of the text.', __METHOD__), E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
2nd -> second ?
Also, do we mention the argument name explicitly in other error messages? If we don't, we could remove the mention to $normalizeWhitespace here. Thanks.
There was a problem hiding this comment.
After a quick search, I found many examples following the same pattern (2nd and the name of the argument). Example: https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/PropertyAccess/PropertyAccessor.php#L855
There was a problem hiding this comment.
That being said, let me know what's the preferred pattern and I'll update this one :)
a7cf1c4 to
f6d8560
Compare
f6d8560 to
54d46ee
Compare
|
@dunglas should this be mentioned in https://github.com/symfony/symfony/blob/master/UPGRADE-4.4.md? I'm just upgrading one of my apps from 4.3 to 4.4 and I only noticed this one via the phpunit bridge as its not mentioned in the upgrade docs |
|
There should be a way to set this (back to) false globally. |
|
A global setting would be the worse API either, because no reusable code could omit the argument as they would not be able to rely on the default value (there is a reason why we deprecated relying on the old default instead of just changing it directly: it changes the behavior) |
According to the DOM and WebDriver specs, browsers always return the normalized text. In Panther, because of WebDriver, it's not even possible without dirty hacks to retrieve the "non normalized" text.
For compatibility with Panther it's mandatory to set this new parameter (introduced in 4.4) to
trueby default.I propose to change the default value to true in 5.0, it has the benefit of:
$node->text()instead of$node->text(null, true), passing true is mandatory because Panther doesn't support retrieving the non-normalized text)For backward compatible with 4.x versions, if no argument is passed and the returned text isn't the same than the normalized one, a notice is triggered.