Project:Drupal.org Project applications
Component:module
Category:task
Priority:normal
Assigned:Unassigned
Status:needs review
Issue tags:PAReview: security

Issue Summary

Note: coerll is applying here for the git vetted user role.

This is a Drupal 6.x project, with plans to port to 7.x as development proceeds.

Purpose

This module allows to annotate the text of a text field with the help of the community tags module and drupal comments. Users can highlight content of a text field in view mode with their mouse cursor and enter a comment or tag in the entry box that pops up.

A word cloud of the entire text, a community tag cloud, all comments, a comment cloud, and a list of annotators are all available through the module as tabbed blocks once the blocks are added to a node_view panel.

Collaborative online annotation offers a new kind of reading experience: instead of making notes in the margin of a book, readers can now share their reactions instantaneously and build a body of commentary about a text together.

Dependencies

- content
- text
- panels
- tagadelic
- taxonomy
- community_tags
- page_manager
- jquery_update
- jquery_ui (with latest jQuery UI 1.7 release)
- views_bonus_export
- comment_delete

The current project page can be found in our sandbox:

http://drupal.org/sandbox/coerll/1782670

The current repository is located at:

git clone --recursive --branch 6.x-1.x git.drupal.org:sandbox/coerll/1782670.git ecomma

Current restrictions:

Does currently not work on IE browsers.
Once a group/class has started to tag or comment on a text the text cannot be changed in any way or the annotations will be off.

Installation:

Instructions below are based on navigation in admin menu (admin_menu module).

1. Site building > Modules: enable ecomma module and dependencies
2. Content management > Content types > Add Content type: Add custom content type.
3. Content management > Content types > Edit Your_custom_content_type > Comment settings > Preview comment: Optional
4. Content management > Content types > Edit Your_custom_content_type > Manage fields > Add a new text area field with unlimited values
5. Enable ecomma commentary features for that text area field in field settings.
6. Content management > Taxonomy: Add Vocabulary, enable Vocabulary for the content type you created, and check Settings > Tags checkbox
7. Site Configurations > Community tags: Enable community tagging for the vocabulary you created
8. Content management > Content types > Edit Your_custom_content_type > Workflow settings > Community tagging form: Select Block
9. User Management > Permissions > community_tags module: Enable "tag content" for authenticated and anonymous users. (This will just enable unauthenticated users to see the tags, but not to tag.)
10. User Management > Permissions > community_tags module: Enable "edit own tags" for authenticated.
11. User Management > Permissions > comment module: Enable "access comments" for authenticated and anonymous users. (This will just enable unauthenticated users to see the comments, but not to comment.)
12. User Management > Permissions > comment_delete module: Enable "delete own comments" for authenticated users.

13. Site building > Pages > List:
-Enable node_view and click on edit.
-Add variant,
-Call it ecomma
-Create variant and enalbe selection rules
-In the selection rules select "Node:type" and then check the content type you created for the ecomma module
-Select miscellaneous layout and choose "eComma column 45/10/45" layout that ships with the ecomma module.

-Add content to Left (click wheel icon): Node > Field: your-text-field - Text > Node being viewed
-Add content to Right: Miscellaneous > Community tagging form
-Add content to Right: Miscellaneous > eComma Tag details
-Add content to Right: Miscellaneous > eComma Word Cloud
-Add content to Right: Miscellaneous > eComma User Annotation Total
-Add content to Right: Node > Comment form > Node being viewed
-Add content to Right: Node > Node Comments
-Add content to Right: Miscellaneous > eComma Comment Cloud

14. Content management > Create content: Create a node for your collaborative text. Don't put anything in the body text just in the text area that you added.

15. Tip: Classes or groups can create collaborative annotation environments with the organic groups module. The userplus module can enhance work with the organic groups module in drupal 6.

15. Current restrictions:
* Does currently not work on IE browser < 9.
* Once a group/class has started to tag or comment on a text the text cannot be changed in any way or the annotations will be off.

PARreview link:

http://ventral.org/pareview/httpgitdrupalorgsandboxcoerll1782670git

Reviews of other projects:

http://drupal.org/node/1913916#comment-7108902
http://drupal.org/node/1924010#comment-7108978
http://drupal.org/node/1924716#comment-7109036

2nd round:
http://drupal.org/node/1943152#comment-7280928
http://drupal.org/node/1939962#comment-7275004
http://drupal.org/node/1953720#comment-7281068
http://drupal.org/node/1962038#comment-7281098

3rd round:
http://drupal.org/node/1967878#comment-7438212

Comments

#1

Adding tag: PAReview: review bonus

#2

Status:needs review» postponed (maintainer needs more info)

I'm confused - who is applying for the git vetted user role here, you or coerll? Because the project is not associated with your account?

Also, "coerll" looks like a group account? Please note: All user accounts are for individuals. Accounts created for more than one user will be blocked when discovered.

#3

coerll is my coworker. Should coerll reapply for the full project?

#4

Status:postponed (maintainer needs more info)» needs review

Switched this to needs review.

#5

Status:needs review» needs work

Ok, I added a note about coerll to the issue summary.

Thank 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).

Review of the 6.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. jquery-ui-1.8.16.custom.min.js: appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.
  2. Please fix the coding standard errors first, so that I can read the code.

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

#6

Forgot attachment.

AttachmentSize
coder-results.txt 204.9 KB

#7

Status:needs work» needs review

Thanks for the review. I cleaned up the code. There is still an e flag in preg_match error even though that doesn't apply to this code, so I decided to ignore it. Any advice? Also found a solution for the jquery ui third party code problem. I used the jquery_ui module instead.
Please take a look and give me feedback whenever you can find some time.

#8

Adding back tag: PAReview: review bonus

#9

Status:needs review» needs work

Review of the 6.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. I'm still not happy that you do commits with the coerll user, which does not look like an individual. Please provide more details who is behind it or create a personalized account that does the commits.
  2. css folder: appers to be third party code that is bundled with jquery UI? Please remove it and tell users where to get it as I said before.
  3. ecomma_init(): why do you add your CSS and JS on every single page request? You should only add it to pages where your functionality is actually used?
  4. ecomma_perm(): permissions are usually lower case and words separated by a single white space.
  5. tag_range_delete/%/%: this path is vulnerable to CSRF exploits. You should be careful with the excution of data changing actions on GET requests, as in this case. See also http://epiqo.com/en/all-your-pants-are-danger-csrf-explained . Please don't remove the security tag, we keep that for statistics and to show examples of security problems.
  6. ecomma_schema(): descriptions on all database fields would be very helpful.
  7. Fatal error: Call to undefined function jquery_ui_add() in ecomma.module on line 30 when I enable the module without enabling the dependencies beforehand. Again the hook_init() problem.
  8. ecomma_tag_query_function(): why don't you use drupal_json() and just pass an array with your values instead of creating your own format with ":"?
  9. ecomma_tag_query_function(): this is vulnerable to SQL injection. You must never pass variables directly to the SQL string, always use placeholders instead. Please read http://drupal.org/writing-secure-code again and all subpages.

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

AttachmentSize
coder-results.txt 421.51 KB

#10

Now actually adding security tag.

#11

Status:needs work» needs review

Thank you so much for your advice and I am sorry it took me so long to take care of the things you mentioned.
I finally understood what you meant with submitting the commits under the wrong name. I had set my username by accident to coerll, so I set it back to my name. I think I took care of all the points you listed and hope you have some time to look at the module again. This module could be really helpful for educational institutions. There are tools like this out there, but not with a visualization component and not easy to adapt for drupal websites.

#12

Status:needs review» needs work

1) use curly brackets in if, foreach etc

<?php
if ($user->uid == 0):
   
$user_role = "anonymous";
  endif;
?>

2) dont add your css, js files in foreach. move it from foreach because files will be added multiple times. (use better lazy loading)
3) In hook_help move your hardcoded html to separate file(like tamplate fille)
4) don't use style="width..." attributes. Use CSS!
5) add comments of function parameters and return values (like in ecomma_comments_active_text_function($beg, $nid) { etc)

#13

6) also use http://ventral.org/ site to review code errors

#14

Status:needs work» needs review

Thanks a.milkovsky.

I took care of point 1 and 3 -5, but I thought lazy loading, as mentioned in point 1 above, is only possible with classes and class objects. So maybe that is possible in d7, but not in d6? Or if it is possible, do you know of a module I could look at as an example. I removed most inline css widths, but I had to leave the display=none in a few instances.