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

Rename default_form_builder to avoid collision #17138

Merged
merged 1 commit into from
Feb 20, 2015
Merged

Rename default_form_builder to avoid collision #17138

merged 1 commit into from
Feb 20, 2015

Conversation

jpcody
Copy link
Contributor

@jpcody jpcody commented Oct 2, 2014

The Configuring Action View guides say the following:

config.action_view.default_form_builder tells Rails which form builder to use by default. The default is ActionView::Helpers::FormBuilder. If you want your form builder class to be loaded after initialization (so it's reloaded on each request in development), you can pass it as a String

Unfortunately, when you set config.action_view.default_form_builder as a string, it is set as a class-level attr_accessor on ActionView::Base. Then line 1191 on master attempts to call an internal method to constantize this string, but instead, it receives the default_form_builder cattr_accessor as a string and attempts to call new on it on the subsequent line.

This problem only appears when setting this configuration before the file is required, and quite honestly, I couldn't discern how to write a test for the case. But as it's only a naming collision, it seemed innocuous enough to submit as a PR.

Thoughts?

@rafaelfranca rafaelfranca merged commit 7c3d25f into rails:master Feb 20, 2015
rafaelfranca added a commit that referenced this pull request Feb 20, 2015
Rename default_form_builder to avoid collision
rafaelfranca added a commit that referenced this pull request Feb 20, 2015
Rename default_form_builder to avoid collision
rafaelfranca added a commit that referenced this pull request Feb 20, 2015
Rename default_form_builder to avoid collision
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.

None yet

2 participants