[D7] Font Icon Select

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxwolffereast2319993git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Automated Review

Pareview.sh is showing some warning - http://pareview.sh/pareview/httpgitdrupalorgsandboxwolffereast2319993git, No a blocker but it would be nice to be fix before the release.

Coder Review

font_icon_select.field.inc

• (*)
      Line 291: Potential problem: form_set_error() and form_error() only accept filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar. (Drupal Docs)
          form_error($element, t('!name field is required.', array('!name' => $element['#title'])));
  

• Line 332: in most cases, replace the string function with the drupal_ equivalent string functions
              '#prefix' => '<span class="font-icon-select-' . $field['settings']['font'] . ' font-icon-select-' . $field['settings']['font'] . '-' . substr($item['value'], 1) . '">',

font_icon_select.admin.inc

• Line 229: in most cases, replace the string function with the drupal_ equivalent string functions
      $key = substr($key, 1);

• Line 332: When labelling buttons, make it clear what the button does, "Submit" is too generic.
      '#value' => t('Submit'),

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No: Does not meets the security requirements.

Coding style & Drupal API usage

List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (*)This project is vulnerable to xss and should pass through check plain. The field font_icon_select_font_library_form:font_title, I'm able to add javascript and execute once I click one of your tab UPLOADED FONT LIBRARIES. This is the same as the first complain from coder, I have manually test it.
2. (+)If there are not any UPLOADED FONT LIBRARIES it would should a message that have a link wrong. Check the attached picture. That's seem like a bug you should make that into a link.
3. It's unclear/confusing, which font format have to be upload it in a order until you try to upload a font and it show and error. The reason why is so confusing because does not have a field description. I think should should add a field description explain what the user needs to upload in each field.
4. Adding the hook_help with some useful information it would be nice for site builders.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Attached picture,

Hi,

1. Use form Api of drupal instead of input tag.
Line no. 455 of font_icon_select.admin.inc

2. use t function in line 533 of of font_icon_select.admin.inc

1. First of all you need to add one parent class with some element in font_icon_select.css .
   eg:--

   div.label {
     display: table-cell;
     font-family: Arial, Helvetica, sans-serif;
     font-size: .9em;
   }

   It can override another label property.

2. Then, It would be better if the *.info file look like this:
   

   name = Font Icon Select
   description = Provides font file management, font based icon select widget, and global/local black/whitelist options for font options.
   core = 7.x
   configure = admin/config/content/font_icon_select_options

   dependencies[] = file

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.