silverstripe / silverstripe-userforms

UserForms module provides a visual form builder for the Silverstripe CMS. No coding required to build forms such as contact pages.
BSD 3-Clause "New" or "Revised" License
135 stars 226 forks source link

TextField adding maxlength="0" attribute by default #367

Closed webdough closed 8 years ago

webdough commented 9 years ago

The TextField is adding maxlength="0" by default, preventing front-end text entry until the "Allowed text length" max value is changed.

adrexia commented 8 years ago

This is still a bug. The default maxlength is set to 0 in the CMS. This is particularly an issue for sites that have upgraded a lot of existing forms, but its also an easy to miss cms field in the case where a user is adding a new form. Suggest not having a maxlength when it has been set to 0, as there isn't really a good use case for having a max length of 0 on a textfield (it would be better to disable the field)

tractorcow commented 8 years ago

Which version is this occurring on? Where did you see this, and on what form? Is it on user-forms on the frontend, or only in the CMS? Is it present in a vanilla install?

From what I can see, 'maxLength' property is set conditionally, only if it's non-empty.

This is the existing code.

/**
 * @return array
 */
public function getAttributes() {
    $maxLength = $this->getMaxLength();

    $attributes = array();

    if($maxLength) {
        $attributes['maxLength'] = $maxLength;
        $attributes['size'] = min($maxLength, 30);
    }

    return array_merge(
        parent::getAttributes(),
        $attributes
    );
}
adrexia commented 8 years ago

I know this is php and it shouldn't matter, but is it being stored as a string?

The reason I know about this bug is because users started complaining that they couldn't fill in a comment field. Changing the maxLength in the CMS fixed the issue. I also saw this happen again yesterday to another developer. If you can't replicate, maybe its a php version thing (though that seems odd)?

Version: yesterday it happened to a project that was working of dev-master, so the latest.

tractorcow commented 8 years ago

Probably stored as an int... but it'll use whatever the user provides in setMaxLength();

I was referring to framework version, rather than PHP version. :) I want to know which version to checkout and test.

edlinklater commented 8 years ago

@tractorcow It has been happening in sites using CWP recipes 1.2.1 and 1.3.0, so that should point you in the right direction of framework and userforms versions.

tractorcow commented 8 years ago

I spent too much time trying to reproduce this and couldn't.

I've thrown up https://github.com/silverstripe/silverstripe-userforms/pull/476. Can someone who can reproduce this error please try it with that PR and report their results?