Skip to content

Hotfix for HTML form attribute handling #15

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

Closed
wants to merge 1 commit into from
Closed

Hotfix for HTML form attribute handling #15

wants to merge 1 commit into from

Conversation

srcspider
Copy link

The following,

$form = $node->ownerDocument->getElementById($formId);

...is an unreliable way to retrieve the ID.

Even if there is an element with an id attribute the id won't be recognized unless the document was validated at the point it was read which in the case of setNode (for whatever reason) does not happen under some circumstances.

To give an example, given the following xml file

<?xml version="1.0" encoding="UTF-8"?>
<root>
    <form id="testform1"></form>
</root>

And the following code,

<?php

ini_set('display_errors',1);
ini_set('display_startup_errors',1);
ini_set('html_errors', 1);
error_reporting(-1);

$doc = new DomDocument;
$doc->validateOnParse = true;
$doc->Load('test.xml');

echo 'Form Tag Exists? '.($doc->getElementById('testform1') === null ? 'Nope' : 'Yup');

The code will still output "Form Tag Exists? Nope" because of how picky the getElementById method is.

For more on the phenomenon see this stackoverflow answer: http://stackoverflow.com/questions/3391942/php-html-domdocument-getelementbyid-problems

I haven't been able to track down why the document in question doesn't work with getElementById, but somehow I'm assuming it has something to do with html5 not having a DTD (though for what it's worth ownerDocument in that particular case somehow gets reduced to just the form and the fields; which is weird in of itself). For reference the real world circumstances where this borks miserably is when using behat with goutte.

The following XPath version appears to reliably work,

$xp = new \DOMXPath($node->ownerDocument);
$form = $xp->query("//*[@id='{$formId}']")->item(0);

…rm tag with an id is reliably retrieved

The following,

$form = $node->ownerDocument->getElementById($formId);

...is an unreliable way to retrieve the ID. 

Even if there is an element with an id attribute the id won't be recognized unless the document was validated at the point it was read which in the case of setNode (for whatever reason) does not happen under some circumstances. 

To give an example, given the following xml file

	<?xml version="1.0" encoding="UTF-8"?>
	<root>
		<form id="testform1"></form>
	</root>

And the following code,

	<?php

	ini_set('display_errors',1);
	ini_set('display_startup_errors',1);
	ini_set('html_errors', 1);
	error_reporting(-1);

	$doc = new DomDocument;
	$doc->validateOnParse = true;
	$doc->Load('test.xml');

	echo 'Form Tag Exists? '.($doc->getElementById('testform1') === null ? 'Nope' : 'Yup');

The code will still output "Form Tag Exists? Nope" because of how picky the getElementById method is.

For more on the phenomenon see this stackoverflow answer: http://stackoverflow.com/questions/3391942/php-html-domdocument-getelementbyid-problems 

I haven't been able to track down why the document in question doesn't work with getElementById, but somehow I'm assuming it has something to do with html5 not having a DTD (though for what it's worth ownerDocument in that particular case somehow gets reduced to just the form and the fields; which is weird in of itself). For reference the real world circumstances where this borks miserably is when using behat with goutte.

The following XPath version appears to reliably work,

	$xp = new \DOMXPath($node->ownerDocument);
	$form = $xp->query("//*[@id='{$formId}']")->item(0);
@fabpot
Copy link
Member

fabpot commented Sep 27, 2013

moved to symfony/symfony#9158

@fabpot fabpot closed this Sep 27, 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.

2 participants