[D7] Multi Text Fields

Update your git clone link to
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/plethoradesign/2140095.git multi_text_fields

I tried to add new field with field type & widget as "multi text field"

Unfortunately i got following error after clicking "Save field settings"

Fatal error: Call to undefined function ckeditor_profiles_compile() in /var/www/drupals/sites/all/modules/multi_text_fields/mtf.module on line 464

in your .module file on line 462 you mentioned
module_load_include('inc', 'ckeditor', 'includes/ckeditor.lib');
this means ckeditor is dependency for this module ? then add dependency in .info file

And
instead of
'#title' => t($settings['title'])
you can use like this
t('@st', array('@st' => $settings['title']))
and also for
'#title' => t($settings['details']),
'#title' => t($settings['additional']),

Hi,

Installed the module, did some functional testing and code review, so here are my findings:

1. Call to undefined function ckeditor_profiles_compile() in mtf.module on line 464. I didn't have CKEditor installed. Either make CKEditor as a requirement or optional.
2. You might want to comment out the hook_views_api() at this point since it's not yet implemented.
3. You might want to remove the function mtf_field_settings_form_validate() if its not gonna be implemented.
4. Function comment for parameters and return value in function mtf_prerender_textarea() should follow the standard:

   @param [type] $parameter
   [description]

   @return
   [description]

5. It is not a good idea to wrap variables with the t(). https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/t/7. Instead, do this t('@title', array('@title' => $settings['title']))

Otherwise, I don't see any other flaws or coding issues that prevents this module from being approved.

I can see the use case - using field_collection is possible, but it would be a lot more work to set up to behave this way. No major issues found in the module, but there are a few style issues reported here: http://pareview.sh/pareview/httpgitdrupalorgsandboxplethoradesign2140095git.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Removing review bonus tag, you have not listed any links to your review comments in the issue summary? See https://drupal.org/node/1975228

fixed links

Thanks you for your reviews. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

manual review:

1. mtf_prerender_textarea(): doc block should use @param and @return style, see https://drupal.org/node/1354#param
2. mtf_field_formatter_view(): don't call theme() here, just return a render array. Drupal will render it later for you.
3. mtf_field_formatter_view(): this is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> as "details" or "additional" I get a nasty javascript popup. You need to sanitize all user provided text before printing, make sure to read https://drupal.org/node/28984 again. You even have text formats available as options, but you never apply them? And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Sorry for the delay. Make sure to review more project applications and get a new review bonus and this will get finished faster.

manual review:

1. "more" is a very bad git commit message, please use https://drupal.org/node/52287
2. mtf_field_instance_settings_form(): you should remove the filter formats here, since they are not used anyway. This is very confusing for users if they can configure something that is not applied in any way.
3. what are the differences to multifield? Please add that to the project page.
4. mtf_fieldoutput(): the XSS filtering should not happen here but rather on the user supplied variables only, for example in mtf_formatter_mtf_default() and the sister functions.
5. mtf_theme(): this hook is useless, since you never specify a render array with #theme using one of them? See https://drupal.org/node/930760 . And you should have a theme_mtf_formatter_mtf_default() function, right? Currenlty the markup generated in mtf_formatter_mtf_default() cannot be overridden by themes, because this is not a real theme function. mtf_field_formatter_view() or mtf_fieldoutput() should return a render array that references your theme function with #theme.

But that are not critical application blockers, so I guess this is RTBC.

Assigning to dman as he might have time to take a final look at this.

no objections for more than a week, so ...

Thanks for your contribution, plethoradesign!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.