Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Conversation

improved-broccoli
Copy link
Contributor

I added a condition when the element is a Captcha. This way, no more validator errors. Just reCaptcha trigger an error validation about the usage of frameborder attribute in iframe tag, but it is reCaptcha related. Not zf2 responsibility.

I didn't write any tests, given the fact it is related to html validation. But I run the usual tests, and nothing goes wrong.

I hope I didn't messed up everything, since it is my first PR.

Tell me if something wrong.

Thanks.
Julien.

@Ocramius
Copy link
Member

I didn't write any tests, given the fact it is related to html validation. But I run the usual tests, and nothing goes wrong.

Requires a new test anyway: nothing gets merged without tests :-\

@improved-broccoli
Copy link
Contributor Author

Well, the test failed on Travis while it was good on my computer. I'm goin' to check that.

{
$captcha = Captcha::factory(array(
'class' => 'dumb'
));
Copy link
Member

Choose a reason for hiding this comment

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

Tests also failed because of CS issues

@improved-broccoli
Copy link
Contributor Author

The test passed on PHP 5.4/5.5/5.6.
Not executed on PHP 5.3 because of an error in AbstractHelperTest:

PHP Parse error:  syntax error, unexpected T_OBJECT_OPERATOR in /home/travis/build/zendframework/zf2/tests/ZendTest/Form/View/Helper/AbstractHelperTest.php on line 41

And partially executed on HHVM because of "Nesting level too deep".
But I think these errors are not related to my commits.

@Ocramius Ocramius self-assigned this Dec 14, 2014
@Ocramius Ocramius added this to the 2.3.4 milestone Dec 14, 2014
Ocramius added a commit that referenced this pull request Dec 16, 2014
Ocramius added a commit that referenced this pull request Dec 16, 2014
Ocramius added a commit that referenced this pull request Dec 16, 2014
Ocramius added a commit that referenced this pull request Dec 16, 2014
@Ocramius Ocramius closed this in 2b4f614 Dec 16, 2014
@Ocramius
Copy link
Member

@jbenoit2011 merged, thanks!

master: 2b4f614
develop: 3330af3

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants