Skip to content

Commit

Permalink
feature #34151 [DomCrawler] normalizeWhitespace should be true by def…
Browse files Browse the repository at this point in the history
…ault (dunglas)

This PR was squashed before being merged into the 4.4 branch (closes #34151).

Discussion
----------

[DomCrawler] normalizeWhitespace should be true by default

| 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

According to the [DOM](https://www.w3.org/TR/DOM-Level-3-Core/core.html#Text3-isElementContentWhitespace) and [WebDriver](https://www.w3.org/TR/webdriver/#get-element-text) 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 `true` by default.

 I propose to change the default value to true in 5.0, it has the benefit of:

* being spec-compliant (in 5.0, text will be normalized by default)
* being cleaner when using Panther (`$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.

Commits
-------

54d46ee [DomCrawler] normalizeWhitespace should be true by default
  • Loading branch information
dunglas committed Oct 29, 2019
2 parents 42be5f8 + 54d46ee commit 3b11a76
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 10 deletions.
Expand Up @@ -29,7 +29,7 @@ public function testFormLoginAndLogoutWithCsrfTokens($config)

$crawler = $client->followRedirect();

$text = $crawler->text();
$text = $crawler->text(null, true);
$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/profile".', $text);

Expand All @@ -56,7 +56,7 @@ public function testFormLoginWithInvalidCsrfToken($config)

$this->assertRedirect($client->getResponse(), '/login');

$text = $client->followRedirect()->text();
$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Invalid CSRF token.', $text);
}

Expand All @@ -75,7 +75,7 @@ public function testFormLoginWithCustomTargetPath($config)

$this->assertRedirect($client->getResponse(), '/foo');

$text = $client->followRedirect()->text();
$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/foo".', $text);
}
Expand All @@ -96,7 +96,7 @@ public function testFormLoginRedirectsToProtectedResourceAfterLogin($config)
$client->submit($form);
$this->assertRedirect($client->getResponse(), '/protected-resource');

$text = $client->followRedirect()->text();
$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/protected-resource".', $text);
}
Expand Down
Expand Up @@ -27,7 +27,7 @@ public function testFormLogin($config)

$this->assertRedirect($client->getResponse(), '/profile');

$text = $client->followRedirect()->text();
$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/profile".', $text);
}
Expand All @@ -47,7 +47,7 @@ public function testFormLogout($config)
$this->assertRedirect($client->getResponse(), '/profile');

$crawler = $client->followRedirect();
$text = $crawler->text();
$text = $crawler->text(null, true);

$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/profile".', $text);
Expand Down Expand Up @@ -80,7 +80,7 @@ public function testFormLoginWithCustomTargetPath($config)

$this->assertRedirect($client->getResponse(), '/foo');

$text = $client->followRedirect()->text();
$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/foo".', $text);
}
Expand All @@ -101,7 +101,7 @@ public function testFormLoginRedirectsToProtectedResourceAfterLogin($config)
$client->submit($form);
$this->assertRedirect($client->getResponse(), '/protected_resource');

$text = $client->followRedirect()->text();
$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Hello johannes!', $text);
$this->assertStringContainsString('You\'re browsing to path "/protected_resource".', $text);
}
Expand Down
12 changes: 10 additions & 2 deletions src/Symfony/Component/DomCrawler/Crawler.php
Expand Up @@ -593,7 +593,7 @@ public function nodeName()
/**
* Returns the text of the first node of the list.
*
* Pass true as the 2nd argument to normalize whitespaces.
* Pass true as the second argument to normalize whitespaces.
*
* @param string|null $default When not null: the value to return when the current node is empty
* @param bool $normalizeWhitespace Whether whitespaces should be trimmed and normalized to single spaces
Expand All @@ -602,7 +602,7 @@ public function nodeName()
*
* @throws \InvalidArgumentException When current node is empty
*/
public function text(/* string $default = null, bool $normalizeWhitespace = false */)
public function text(/* string $default = null, bool $normalizeWhitespace = true */)
{
if (!$this->nodes) {
if (0 < \func_num_args() && null !== func_get_arg(0)) {
Expand All @@ -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 second "$normalizeWhitespace" argument to false to retrieve the non-normalized version of the text.', __METHOD__), E_USER_DEPRECATED);
}

return $text;
}

if (\func_num_args() > 1 && func_get_arg(1)) {
return trim(preg_replace('/(?:\s{2,}+|[^\S ])/', ' ', $text));
}
Expand Down
Expand Up @@ -258,6 +258,14 @@ public function testNormalizeWhiteSpace()
$crawler = $this->createTestCrawler()->filterXPath('//p');
$this->assertSame('Elsa <3', $crawler->text(null, true), '->text(null, true) returns the text with normalized whitespace');
$this->assertNotSame('Elsa <3', $crawler->text(null, false));
}

/**
* @group legacy
*/
public function testLegacyNormalizeWhiteSpace()
{
$crawler = $this->createTestCrawler()->filterXPath('//p');
$this->assertNotSame('Elsa <3', $crawler->text());
}

Expand Down

0 comments on commit 3b11a76

Please sign in to comment.