8000 [DomCrawler] fixed HTML5 form attribute handling by kepten · Pull Request #8197 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DomCrawler] fixed HTML5 form attribute handling #8197

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

Closed
wants to merge 3 commits into from
Closed

[DomCrawler] fixed HTML5 form attribute handling #8197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
181cddd
fixed HTML5 form attribute handling and added some tests
kepten Jun 4, 2013
787b4ec
removed an unnecessary condition check
kepten Jun 4, 2013
38bb4d3
changed code to improve performance
kepten Jun 5, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
8000 Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 31 additions & 18 deletions src/Symfony/Component/DomCrawler/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,46 +366,59 @@ protected function setNode(\DOMNode $node)
$this->node = $node;
}

/**
* Adds form elements related to this form.
*
* Creates an internal copy of the submitted 'button' element and
* the form node or the entire document depending on whether we need
* to find non-descendant elements through HTML5 'form' attribute.
*/
private function initialize()
{
$this->fields = new FormFieldRegistry();

$document = new \DOMDocument('1.0', 'UTF-8');
$node = $document->importNode($this->node, true);
$button = $document->importNode($this->button, true);
$root = $document->appendChild($document->createElement('_root'));
$root->appendChild($node);
$root->appendChild($button);
$xpath = new \DOMXPath($document);
$root = $document->appendChild($document->createElement('_root'));

// add descendant elements to the form
$fieldNodes = $xpath->query('descendant::input | descendant::button | descendant::textarea | descendant::select', $root);
foreach ($fieldNodes as $node) {
$this->addField($node, $button);
// add submitted button if it has a valid name
if ($this->button->hasAttribute('name') && $this->button->getAttribute('name')) {
$this->set(new Field\InputFormField($document->importNode($this->button, true)));
}

// find form elements corresponding to the current form by the HTML5 form attribute
// find form elements corresponding to the current form
if ($this->node->hasAttribute('id')) {
// traverse through the whole document
$node = $document->importNode($this->node->ownerDocument->documentElement, true);
$root->appendChild($node);

// corresponding elements are either descendants or have a matching HTML5 form attribute
$formId = Crawler::xpathLiteral($this->node->getAttribute('id'));
$xpath = new \DOMXPath($this->node->ownerDocument);
$fieldNodes = $xpath->query(sprintf('descendant::input[@form=%s] | descendant::button[@form=%s] | descendant::textarea[@form=%s] | descendant::select[@form=%s]', $formId, $formId, $formId, $formId));
$fieldNodes = $xpath->query(sprintf('descendant::input[@form=%s] | descendant::button[@form=%s] | descendant::textarea[@form=%s] | descendant::select[@form=%s] | //form[@id=%s]/input[not(@form)] | //form[@id=%s]/button[not(@form)] | //form[@id=%s]/textarea[not(@form)] | //form[@id=%s]/select[not(@form)]', $formId, $formId, $formId, $formId, $formId, $formId, $formId, $formId), $root);
foreach ($fieldNodes as $node) {
$this->addField($node, $button);
$this->addField($node);
}
} else {
// parent form has no id, add descendant elements only
$node = $document->importNode($this->node, true);
$root->appendChild($node);

// descendant elements with form attribute are not part of this form
$fieldNodes = $xpath->query('descendant::input[not(@form)] | descendant::button[not(@form)] | descendant::textarea[not(@form)] | descendant::select[not(@form)]', $root);
foreach ($fieldNodes as $node) {
$this->addField($node);
}
}
}

private function addField(\DOMNode $node, \DOMNode $button)
private function addField(\DOMNode $node)
{
if (!$node->hasAttribute('name') || !$node->getAttribute('name')) {
return;
}

$nodeName = $node->nodeName;

if ($node === $button) {
$this->set(new Field\InputFormField($node));
} elseif ('select' == $nodeName || 'input' == $nodeName && 'checkbox' == $node->getAttribute('type')) {
if ('select' == $nodeName || 'input' == $nodeName && 'checkbox' == $node->getAttribute('type')) {
$this->set(new Field\ChoiceFormField($node));
} elseif ('input' == $nodeName && 'radio' == $node->getAttribute('type')) {
if ($this->has($node->getAttribute('name'))) {
Expand Down
48 changes: 40 additions & 8 deletions src/Symfony/Component/DomCrawler/Tests/FormTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,53 @@ public function testConstructorHandlesFormAttribute()
$dom = new \DOMDocument();
$dom->loadHTML('
<html>
<form id="bar">
<input type="submit" form="bar" />
<form id="form_1" action="" method="POST">
<input type="checkbox" name="apples[]" value="1" checked />
<input form="form_2" type="checkbox" name="oranges[]" value="1" checked />
<input form="form_1" type="hidden" name="form_name" value="form_1" />
<input form="form_1" type="submit" name="button_1" value="Capture fields" />
<button form="form_2" type="submit" name="button_2">Submit form_2</button>
</form>
<input form="form_1" type="checkbox" name="apples[]" value="2" checked />
<form id="form_2" action="" method="POST">
<input type="checkbox" name="oranges[]" value="2" checked />
<input type="checkbox" name="oranges[]" value="3" checked />
<input form="form_2" type="hidden" name="form_name" value="form_2" />
<input form="form_1" type="hidden" name="outer_field" value="success" />
<button form="form_1" type="submit" name="button_3">Submit from outside the form</button>
</form>
<input type="submit" form="bar" />
<button />
</html>
');

$nodes = $dom->getElementsByTagName('input');
$inputElements = $dom->getElementsByTagName('input');
$buttonElements = $dom->getElementsByTagName('button');

$form = new Form($nodes->item(0), 'http://example.com');
$this->assertSame($dom->getElementsByTagName('form')->item(0), $form->getFormNode(), 'HTML5-compliant form attribute handled incorrectly');
// Tests if submit buttons are correctly assigned to forms
$form1 = new Form($buttonElements->item(1), 'http://example.com');
$this->assertSame($dom->getElementsByTagName('form')->item(0), $form1->getFormNode(), 'HTML5-compliant form attribute handled incorrectly');

$form1 = new Form($inputElements->item(3), 'http://example.com');
$this->assertSame($dom->getElementsByTagName('form')->item(0), $form1->getFormNode(), 'HTML5-compliant form attribute handled incorrectly');

$form2 = new Form($buttonElements->item(0), 'http://example.com');
$this->assertSame($dom->getElementsByTagName('form')->item(1), $form2->getFormNode(), 'HTML5-compliant form attribute handled incorrectly');

// Tests if form elements are correctly assigned to forms
Copy link
Member

Choose a reason for hiding this comment

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

If you test different things, you should have different tests instead of more assertions in the same test. It will give a better output in case of failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point but I think it tests the same thing (form attribute handling) just in two different ways.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kepten I know it's merged already but for future - the very fact you needed to add comments to group your tests implies they're different cases.

$values1 = array(
'apples' => array('1', '2'),
'form_name' => 'form_1',
'button_1' => 'Capture fields',
'outer_field' => 'success'
);
$values2 = array(
'oranges' => array('1', '2', '3'),
'form_name' => 'form_2',
'button_2' => '',
);
$this->assertEquals($values1, $form1->getPhpValues(), 'HTML5-compliant form attribute handled incorrectly');
$this->assertEquals($values2, $form2->getPhpValues(), 'HTML5-compliant form attribute handled incorrectly');

$form = new Form($nodes->item(1), 'http://example.com');
$this->assertSame($dom->getElementsByTagName('form')->item(0), $form->getFormNode(), 'HTML5-compliant form attribute handled incorrectly');
}

public function testMultiValuedFields()
Expand Down
0