Image ALT text

This module make the alt or title text mandatory on widget type image.

Project page link

    https://www.drupal.org/sandbox/prajaankit/2486011

Git Clone command

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/prajaankit/2486011.git image_alt_text

Features

  • This module make the alt or title text mandatory on widget type image

Goals

  • This module make the alt or title text mandatory on widget type image

Manual reviews of other projects:

Comments

PA robot’s picture

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.

prajaankit’s picture

Issue summary: View changes
PA robot’s picture

Status: Active » Needs work

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

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

Vikas.Kumar’s picture

There are some issue inside module info file as well you need to use proper commenting in module file for each function. Add readme .txt file for use condition for module.

rajesh.vishwakarma’s picture

Automated Review

Your module have some error that reported by the automated system. Those need to fix.
http://pareview.sh/pareview/httpgitdrupalorgsandboxprajaankit2486011git

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.

No duplication
Yes: It has additional integration with views also available statics in our system.

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 code.

README.txt/README.md
Yes: Follows the guidelines for in-project documentation

Code long/complex enough for review
No: Please check guidelines for project length.

Secure code
Yes: Meets the security requirements

Coding style & Drupal API usage
1) First fixed coding format as reported by http://pareview.sh/pareview/httpgitdrupalorgsandboxprajaankit2486011git

2) Replace core = 7.x-1.x with core = 7.x in image_alt_text.info file, due to this line module display This version is not compatible with Drupal 7.x and should be replaced. checked VERSION 7.34.

3) This module restrict for all images field required even where I don't need to make mandatory.

4) How can I make only single (title or alt) field required. There should be settings like check box with both field show that user can manage easily.

prajaankit’s picture

Thank rajesh for your valueable comment

we remove all the coding standard error

prajaankit’s picture

Issue summary: View changes
Status: Needs work » Needs review
prajaankit’s picture

Issue summary: View changes
prajaankit’s picture

Issue summary: View changes
prajaankit’s picture

Issue summary: View changes
prajaankit’s picture

Issue summary: View changes
darol100’s picture

Status: Needs review » Needs work
Issue tags: +PAreview: security

@prajaankit,

  • I would have to agree with #5
    This module restrict for all images field required even where I don't need to make mandatory.
  • Coder show some errors:
    Line 54: 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. 
    
          form_error($element, t('The field !title is required', array('!title' => $element['#title'])));
    
    Line 76: 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. 
    
          form_error($element, t('The field !title is required', array('!title' => $element['#title'])));

    these are security issue. For this reason this is a blocker. You need to fix this.

  • There is a typo on line 9 because it said hook_hook_field_widget_form_alter instead of hook_field_widget_form_alter
  • I think this should be a patch as an option in the image field module. This module is too short, would you be interested on a single project promote?
dydave’s picture

Hi guys,

Stumbled on the module at random.

First of all @prajaankit, Thanks a lot for making this step forward, towards contributing Drupal projects. That's the spirit! It's definitely greatly appreciated.

Without even going into PAReview, automated testing, security issues or code checking, I would have the following comments:

1 - Duplication
Isn't this module actually duplicating a feature/functions supposed to be part of the Accessible module?
See #887424: Make alt text required on imagefields and #815144-5: Imagefields should have the option to require Custom Alt text, also related to this feature: #2303765: Make the default 'alt' attribute for Image fields required and #347070: Ability to make alt and title text required for each image.

and what about the Extend Image Module?

It would have been nice if you could have given some details about where this requirement came from (related to an issue on Drupal.org, for example) and why it was decided to be developed as a module instead of a feature part of another module.

Wouldn't it be better to join your forces/efforts with the Accessible module (which by the way needs help and a co-maintainer) to get this pushed in the module as a patch, rather than duplicating the feature in another module...
which by the way doesn't even contain 120 lines of processing code/functions.... (2 - Code too short)

In my opinion, these 2 points above would seem to be critical enough to actually reject your project application.

Could you please bring a solid and robust justification of why this feature should be a stand-alone/independant module? Isn't there any other module it could be attached to/part of? Could you please share the results of your research with us (links, issues, etc...) whatever you found related with the duplication of this feature)?

3 - Project page needs improvement
On top of that, Image ALT text project page is really poor... I mean, two lines of information?! ... that's very very far from what would actually be expected for a module going through this validation process and best practices in general (if you would like anybody to actually really use your contributed modules).

If you are serious about contributing modules and becoming a vetted Drupal developer, I would strongly recommend that you take a closer look, at least once, at the following documentation:

 
My personal advice: drop this project application and try to get this pushed in Accessible as a co-maintainer.
Get a bit more practice as a co-maintainer and get back to project application after a while with another module, better prepared.

Just my 2 cents. Hope that helps, even though that's probably not what you want to hear.... I prefer to share my honest opinion.
Thanks again very much for your contributions, positive attitude and involvement in the Drupal developers community.
Cheers!

darol100’s picture

Issue tags: -PAreview: security

Removing the PAReview: security after some testing this does not see to be an security issue.

PA robot’s picture

Status: Needs work » Closed (won't fix)

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.