The Voice Commander module automatically makes voice commands for navigating the entire menu system of your Drupal site, using Web Speech API.

You can choose which menu to work, than just press Ctrl+Alt and start speak. Also, you can turn on voice commands on mobile device.
Now Google's voice recognition service is gaining popularity and may already be comfortable enough to use in Chrome on desktops and mobile devices.

Ease of operation is achieved through the use of voice instead of manual navigation.
Voice commander has no analogues in Drupal world. It works with Web Speech API that already builded in Chrome Browser and and has already begun to develop maintaint of Speech in Safari.

Project: https://www.drupal.org/sandbox/yuriy.kostin/2385113

Git clone command: git clone --branch 7.x-1.x http://git.drupal.org/sandbox/yuriy.kostin/2385113.git voicecommander

PAReview link: http://pareview.sh/pareview/httpgitdrupalorgsandboxyuriykostin2385113git

Manual reviews of other projects:

Comments

yuriy.kostin’s picture

Issue summary: View changes
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.

alexander_danilenko’s picture

Status: Needs review » Needs work

There is no functionality related to error reporting to end-user. For example if command is missing or don't allowed - user can't see any indications or errors on page.

Also there is no command for clearing cache.

andrdrx’s picture

Added handlers and set messages for errors, wrong commands or unexpected situations.

yuriy.kostin’s picture

Status: Needs work » Needs review
artsakenos’s picture

Status: Needs review » Needs work

Hello Yuriy, Yaremiy, Alexandand, a nice contribution indeed, I hope a functionality like this can become quickly available for the Drupal community. At the moment I found many issues during the installation and usage process. My suggestions follow:

Automated Review
⊠ Git default branch is not set, see the documentation on setting a default branch.

Individual user account
☑ Follows the guidelines for individual user accounts.

No duplication
☑ Does not cause module duplication and/or fragmentation
Except the unmantained https://www.drupal.org/project/speech I see no similar solutions available for the community.

Master Branch
✎ Follows the guidelines for master branch. But a default branch is not set up, please check the pareview output.

README.txt/README.md
✎ The content itself is ok and follows the guidelines for in-project documentation and/or the README Template.
Just take care the underlining must everywhere be as long as the title.

Installation / Usage
⊠ I experienced issues during the installation with drush:
Fatal error: Call to undefined function drush_voicecommander_download() in /home/drupalpro/websites/merda.dev/sites/all/modules/custom/voicecommander/drush/voicecommander.drush.inc on line 113

After the installation the website warns you constantly:
VoiceCommander: plugin "Annyang" or file "annyang.min.js" is not exist in libraries folder.
Instead, once the module is installed I would send a notification to the user to warn him to install the libraries, providing links as support.

Also after having installed the warning remains until a cache clearing is performed.

While using it, it always appear the Allow Deny dialog box, which is annoying, and anyway it doesn’t catch what I say, even selecting all the menu. I am not sure which command list Is available, I would highlight it somewhere, because right now I couldn't perform any command. This was the result for me more or less: http://youtu.be/lu88J5JL8Hw

Project Description
⊠ The title of this issue is wrong, it should be [D7] VoiceCommander instead.

The git command is wrong,
git clone --branch 7.x-1.x yuriy.kostin@git.drupal.org:sandbox/yuriy.kostin/2385113.git voicecommander
should be instead
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/yuriy.kostin/2385113.git voicecommander

Also, you should provide the pareview link in the issue page to make the review process easier.
in this case: http://pareview.sh/pareview/httpgitdrupalorgsandboxyuriykostin2385113git

Good Luck with this amazing module!

andrdrx’s picture

Title: VoiceCommander [D7] » [D7] VoiceCommander
yuriy.kostin’s picture

Issue summary: View changes
andrdrx’s picture

Hello Andrea and thank you for your review of our first module. We have reviewed your comments and made next fixes and updates;

  1. Automated Review: Default branch set. Now it is 7.x.-1.x
  2. Master Branch: Master branch set. Now it is 7.x.-1.x
  3. README.txt: Fixed underlines length.
  4. Installation / Usage: Update user notification in case if library is not installed. Fixed drush post-install behavior: now we automatically clean cache after install. Soon we will provide highlighting of allowed commands functionality. Now we have next default commmands:
    • Cache clear: 'drupal cache'
    • Management menu: '[Administartor command prefix] [Management menu item]', where [Administartor command prefix] is an option that set at /admin/config/user-interface/voice-commander and [Management menu item] is an any menu link from management menu /admin/structure/menu/manage/management
  5. Project Description: Updated project documentations.
yuriy.kostin’s picture

Status: Needs work » Needs review
yuriy.kostin’s picture

Issue summary: View changes
mavimo’s picture

Status: Needs review » Needs work
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. (*) On voicecommander.module comment refer to Colorbox module
  2. (+) Private function have missing docs.
  3. (+) voicecommander.module, on line 75 there is drupal_set_message where the URL is defined above, and is replicate. User placeholder to replace it.
  4. (+) voicecommander.module, on line 97 and 98 there is variable_get without default value.
  5. Are you sure you need to expose the CC all through the AJAX callback?
  6. On JS side: typeof (annyang) !== 'undefined' can be undefined !== annyang
  7. (+) On JS side: config with hardcoded string is bad, please export it using drupal_add_js('settings', ..) and load this value drop Drupal.settings.voicecommander.....
  8. (+) On JS side: limitate all DOM interaction to context param eg $('.my-selector', context).

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.

This review uses the Project Application Review Template.

PS: nice work!

yuriy.kostin’s picture

Issue summary: View changes
yuriy.kostin’s picture

Issue summary: View changes
yuriy.kostin’s picture

Issue summary: View changes
joachim’s picture

Issue tags: -voice, -commander, -voice commander, -blink reaction

This looks really cool!

I had a few teething problems though:

- Chrome asks for my permission to use the microphone every single time I held down CTRL+ALT. This makes it really hard to use.
- None of my commands were recognized at first. It's not clear from the readme what sort of things I can say. Some examples would be good, eg: 'Drupal Content Types', 'Drupal Modules', etc.

On to a code review:

There are a few mentions of another module, such as:

/**
 * The default path to the Colorbox directory.
 */

Repeating the name of the function doesn't count as documentation. You should state what the function does:

 * Custom function _voicecommander_base_menu_recursive().

You also need to document the @params and @return.

function voicecommander_settings_form() {

Form builder functions have standard params which are missing here.

Providing a drush command is a very nice touch, though it does this:

$ drush vc
Command vc needs the following module(s) enabled to run:             [error]
voicecommander.
The drush command 'vc' could not be executed.

The README should have instructions on where to get the library from.

> Select menu for commands *(Management menu use default)

I don't understand what this UI text means at all.

The module description in the info is rather long -- some of that should go in a hook_help().

function _voicecommander_cc_all() {
  drupal_flush_all_caches();
}

Not sure what this is for...
If you want to provide a cache clear command, would the admin_menu menu item not do?

BTW, please don't tag issues with random keywords! Read the guidelines that are linked on the tag field.

yuriy.kostin’s picture

Status: Needs work » Needs review

Hi Marco and Joachim, thank you for your review comments and your time.

Answers on Joachim questions:

About “Chrome asks for my permission”

- Chrome will be asking for your permission to use the microphone every single time while you are using http:// instead https://. If you're site is using ssl permission to use the mic will be ask once. This is non-secure protocol issue. Also we have already described this behavior in readme file and module description/

About “None of my commands were recognized”

You must say menu item titles as navigation commands. For example:
- you have Main Menu on youre site, and "Home" item in this menu.
- you have choosed Main Menu on VoiceCommander settings page.
- in this way you can just say "Home" and VoiceCommander will redirect you to Home Page.

About Drush commands

At first you need enable module:
$ drush en voicecommander -y
And then use command for install Annyang plugin:
$ drush vc

joachim’s picture

> Also we have already described this behavior in readme file and module description

I eventually found that, but it's nested in the README several folders down. Would be better in the top-level README.

yuriy.kostin’s picture

Issue summary: View changes
yuriy.kostin’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus
klausi’s picture

Issue summary: View changes
Issue tags: -PAReview: review bonus

Removing review bonus tag, you have not done all manual reviews, you just posted the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.

joachim’s picture

Status: Needs review » Needs work

This is breaking normal JS behaviour on my development site:

- I am working on https://www.drupal.org/project/reference_option_limit, and with VC enabled the JS on the field widget only works the first time.
- Module Filter doesn't work at all

andrdrx’s picture

Status: Needs work » Needs review
Issue tags: +javascript scope

Hello, Joachim and thank you for your time to view our project. We figured out the reason of error and have fixed it. For both cases described by you it was same root cause.

yuriy.kostin’s picture

Issue tags: -javascript scope
yuriy.kostin’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus
klausi’s picture

Assigned: Unassigned » naveenvalecha
Status: Needs review » Needs work
Issue tags: -PAReview: review bonus +PAReview: security

manual review:

  1. "$t('@e', array('@e' => $error_type)),": that translatable string is not helpful, what would be a meaningful translation if it only consists of the placeholder? Just use check_plain() on $error_type here.
  2. voicecommander_preprocess_page(): since you are not using $variables here you should use hook_page_build() instead to add your JS. And there you should use #attached on the $page render array instead of drupal_add_js(), see https://api.drupal.org/api/drupal/developer--topics--forms_api_reference...
  3. There is a security issue in this module, so I'm assigning this to naveenvalecha as part of our git admin security training program. If Naveen or Yuriy is not able to find it I will post the details in one week. 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.

klausi’s picture

JS: window.href uses / instead of Drupal.basePath or whatever it is called.

naveenvalecha’s picture

Assigned: naveenvalecha » Unassigned

Manual Review Review of the 7.x-1.x branch (commit dedc4e7):

voicecommander.module : function _voicecommander_cc_all : drupal_flush_all_caches() is very heavy operation.As I have seen that you are calling from the voicecomamnder.js file.For long term, I would suggest please specify about it on the project page and also add the comments in the code.

  $items['cc_all'] = array(
    'title' => 'clear cache',
    'type' => MENU_NORMAL_ITEM,
    'page callback' => '_voicecommander_cc_all',
    'access arguments' => array('access voice commander settings'),
  );

Do we need the menu type "MENU_NORMAL_ITEM" . Please add a comment.
Also I suggest that menu path would be 'admin/config/....' instead of "cc_all" menu item path to . Its just suggestion.

2)
use Drupal.settings.basePath instead of / as @klausi addressed above.

3)
voicecommander.admin.inc function voicecommander_settings_form : Spelling issue, "administartor".
Security Issues :
1)
voicecommander.module function _voicecommander_base_menu_recursive: This function has an access by pass problem.So, users may see node titles and other menu entries that that may not have access to, which is a security problem. You need to call menu_link_load() on each item and then check access. This step will also translate the menu title.
Similarly in the _voicecommander_menus_recursive function.
2)
voicecommander.admin.inc function voicecommander_settings_form : The menu_get_menus function is displaying the Menus in the select list. This is displaying all the menus here.So this function will also display the custom menus to the user with "access voice commander settings" can see all the menus type even the user does not have "administer menu settings" permissions which seems a light security issue, so need some access check here as well.
Edit : Addressed by @Klausi to me on IRC. Thanks :)
3)

  $items['cc_all'] = array(
    'title' => 'clear cache',
    'type' => MENU_NORMAL_ITEM,
    'page callback' => '_voicecommander_cc_all',
    'access arguments' => array('access voice commander settings'),
  );

The path is also vulnerable to CSRF exploits.so either needs a confirmation form / security token in the url.See http://epiqo.com/en/all-your-pants-are-danger-csrf-explained for more information.

klausi’s picture

I don't think it is necessary to call menu_link_load() on every menu item, the API function should return the #access key for each item which indicates whether the user is allowed to see the item or not.

klausi’s picture

As I discussed with Naveen on IRC another security issue is the cc_all path. This is vulnerable to CSRF exploits because you are performing write operations without a confirmation form or a security token in the URL. See http://epiqo.com/en/all-your-pants-are-danger-csrf-explained and https://docs.acquia.com/articles/protecting-your-drupal-module-against-c...

kevinus’s picture

Hello Naveenvalecha and Klausi,

About:

Security Issues :
1)
voicecommander.module function _voicecommander_base_menu_recursive: This function has an access by pass problem.So, users may see node titles and other menu entries that that may not have access to, which is a security problem. You need to call menu_link_load() on each item and then check access. This step will also translate the menu title.
Similarly in the _voicecommander_menus_recursive function.

Function menu_tree_all_data() is already have "security check". Look at include/menu.inc (line 1150 and line 1355).
I see no reason to double check for access.

Same as for

2)
voicecommander.admin.inc function voicecommander_settings_form : The menu_get_menus function is displaying the Menus in the select list. This is displaying all the menus here.So this function will also display the custom menus to the user with "access voice commander settings" can see all the menus type even the user does not have "administer menu settings" permissions which seems a light security issue, so need some access check here as well.

yuriy.kostin’s picture

Status: Needs work » Needs review

Hi Klausi and Naveen,
Thank of review and your time.

About Klausi issues:
1. Fixed
2. Fixed
JS: Fixed (used Drupal.settings.basePath instead of / a)

About Naveen issues: Manual Review is all fixed

Security Issues :
1. See Kevinus answer https://www.drupal.org/node/2391837#comment-9590601
2. See Kevinus answer https://www.drupal.org/node/2391837#comment-9590601
3. Fixed

yuriy.kostin’s picture

Issue summary: View changes
yuriy.kostin’s picture

Issue summary: View changes
yuriy.kostin’s picture

Issue summary: View changes
Issue tags: +PAReview: review bonus
naveenvalecha’s picture

Review of the 7.x-1.x branch (commit ba3d08a)

  1. voicecommander_preprocess_page(): since you are not using $variables here you should use hook_page_build() instead to add your JS. And there you should use #attached on the $page render array instead of drupal_add_js(), see https://api.drupal.org/api/drupal/developer--topics--forms_api_reference...
  2. _voicecommander_cc_all_form(): The organization I worked with we usually add the prefix to functions which are the helper function and will be callled in module internally.
  3. _voicecommander_cc_all_form(): $_GET['destination'].$_GET is fine but drupal_get_query_parameters function is preferable.
  4. _voicecommander_cc_all_form_submit(): drupal_goto(); breaks the other processing because it do 30X so $form_state['redirect'] is prefferable.
  5.  $items['admin/config/user-interface/voice-commander/cc_all'] = array(
        'title' => 'clear cache',
        'access arguments' => array('access voice commander settings'),
        'page callback' => 'drupal_get_form',
        'page arguments' => array('_voicecommander_cc_all_form', 1),
        'access callback' => TRUE,
        'type' => MENU_CALLBACK,
      );

    The second page argument is not using in the callback function ? If not then simply remove.

  6. It looks _voicecommander_cc_all_form_submit will not be called using Js function clearCache because the confirm form will not be clicked automatically using js call at the menu path.Please add a comment ?

Waiting just for my last point.Otherwise RTBC :) Thanks for your help in reviewing other project applications and patience in the Project applications review process.

klausi’s picture

Assigned: Unassigned » Dave Reid
Status: Needs review » Reviewed & tested by the community

Repeating myself from the last review:

  • "$t('@e', array('@e' => $error_type)),": that translatable string is not helpful, what would be a meaningful translation if it only consists of the placeholder? Just use check_plain() on $error_type here.

+ the things Naveen mentioned. But that are not application blockers, otherwise looks RTBC to me.

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

yuriy.kostin’s picture

Hi Naveen and Klaus,

All your issues of comments #36 and #37 are fixed.

Thank you for your help with review module and your time!

klausi’s picture

Assigned: Dave Reid » Unassigned
Status: Reviewed & tested by the community » Fixed

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

Thanks for your contribution, yuriy.kostin!

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:

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.

yuriy.kostin’s picture

Thanks to all who's helped review of this module!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.