Skip to content
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

[BUG] $form->add() stopped working, latest Phalcon 1.3.0 #1852

Closed
temuri416 opened this issue Jan 18, 2014 · 13 comments
Closed

[BUG] $form->add() stopped working, latest Phalcon 1.3.0 #1852

temuri416 opened this issue Jan 18, 2014 · 13 comments

Comments

@temuri416
Copy link
Contributor

I've just compiled the latest Phalcon 1.3.0.

As a result, my forms stopped working:

Unexpected value type: expected object implementing Phalcon\Forms\ElementInterface, object of type KR\Forms\Element\Html given
class FormRegAccountAdmin extends Phalcon\Forms\Form
{
    /**
     * @param array $params Form instantiation parameters
     */
    public function __construct(array $params = null)
    {
        $el = new Html('email');
        $el->setLabel($l='Email')
            ->setAttributes([
                'placeholder' => $l,
            ]
        );
        $this->add($el); // <----- THROWS ERROR
    }
}
use
    Phalcon\Forms\Element;

class Html extends Element
{
    /**
     * Render element.
     *
     * @return string
     */
    public function __toString()
    {
        return $this->getValue();
    }
}

As you can see, I'm adding Html object which extends Phalcon's native form element, thus inheriting the "implements" property.

Thanks!

@temuri416
Copy link
Contributor Author

Looks like Phalcon\Forms\Element no longer implements Phalcon\Forms\ElementInterface.

@ghost
Copy link

ghost commented Jan 18, 2014

This is true for many classes. If there is a class Xxx which is a base for Yyy and Zzz, Xxx itself never implements XxxInterface, only its descendants do.

Quick solution is obviously

-class Html extends Element
+class Html extends Element implement Phalcon\Form\ElementInterfce

@ghost
Copy link

ghost commented Jan 18, 2014

@phalcon

Are there any objections if we make base classes implement corresponding interfaces?

ie,

Phalcon\Annotations\Adapter => Phalcon\Annotations\AdapterInterface
Phalcon\Cache\Backend => Phalcon\Cache\BackendInterface
Phalcon\Cache\Frontend => Phalcon\Cache\FrontendInterface

etc?

@temuri416
Copy link
Contributor Author

did you change something recently? it was fine up until 1 hr ago when I recompiled Phalcon.

@ghost
Copy link

ghost commented Jan 18, 2014

Don't think so, I was playing with phalcon_call_XXX functions, they are unrelated to interfaces. At least I do not remember adding PHALCON_VERIFY_INTERFACE macros recently.

@temuri416
Copy link
Contributor Author

well... this is most unfortunate. all my forms are suddenly crashing. what would you suggest?

@ghost
Copy link

ghost commented Jan 18, 2014

Please apply this patch and recompile Phalcon:

diff --git a/ext/forms/element.c b/ext/forms/element.c
index 9ed7782..d67fa4c 100644
--- a/ext/forms/element.c
+++ b/ext/forms/element.c
@@ -137,6 +137,8 @@ PHALCON_INIT_CLASS(Phalcon_Forms_Element){
        zend_declare_property_null(phalcon_forms_element_ce, SL("_options"), ZEND_ACC_PROTECTED TSRMLS_CC);
        zend_declare_property_null(phalcon_forms_element_ce, SL("_messages"), ZEND_ACC_PROTECTED TSRMLS_CC);

+       zend_class_implements(phalcon_forms_element_ce TSRMLS_CC, 1, phalcon_forms_elementinterface_ce);
+
        return SUCCESS;
 }

@temuri416
Copy link
Contributor Author

great, thanks. this is not a temporary patch, and it will go into repo, right?

@phalcon
Copy link
Collaborator

phalcon commented Jan 18, 2014

@sjinks no problem, go ahead 👍

@ghost
Copy link

ghost commented Jan 18, 2014

@temuri416 yes, I will submit the patch in a couple of hours — I am in the middle of another thing at the moment.

@temuri416
Copy link
Contributor Author

Thanks, much appreciated.

@temuri416
Copy link
Contributor Author

@sjinks That diff you pasted above - what's the tool that can apply the change?

@ghost
Copy link

ghost commented Jan 18, 2014

git apply

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

No branches or pull requests

2 participants