[D7] disable js

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.

Thank you!
Please remove the master branch

Just a small improvement for your hook_uninstall implementation.
Function variable_del() has no hooks and the only thing it does is a DELETE query plus a cache clear (which is not inexpensive).
Your module creates 3 variables per theme; suppose you have 6 themes installed, this gives 18 variables so 18 queries to delete all of them (and 18 useless cache clears).

I suggest to delete all of them in a single query then clear cache only once, like this:

/**
 * Implements hook_uninstall().
 */
function disable_js_uninstall() {
  db_delete('variables')
    ->condition('v.name', 'disable_js_%', 'LIKE')
    ->execute();
  cache_clear_all('variables', 'cache_bootstrap');
}

I attached my patch for this.

Best wishes for your module.

None of these are application blocker however its good to make these quick changes

• README.txt : Please follow the guideline for readme.txt file & provide more useful information.
• You have mentioned a configure link in INFO file. However this link will lead the user only to theme list page. Some users might get confused. My suggestion is to remove that.
• nicorac have a valid point (as per comment #4) i suggest you to incorporate that change. But its not a blocker
• Update your sandbox link to the project description page

Please manually review 3-4 Projects & have a review bonus tag added to your application. Project applications with review bonus always get priority
Also please go thru the application submission checklist which would be of great help.
Additionally you can also check Apply for permission to create full projectspage for clear instruction on how to have your project description updated.

saurabh-chugh, i'm not seeing any blocker issue on your module. Its Good to RTBC. However i request you to take care those items from #5 before releasing your module.

Automated Review

No issues.

Manual Review

(*) Your project page is not very detailed, please have a look at the tips for a great project page, you may also use HTML-tags for better structure.

(*) Please create a README.txt that follows the guidelines for in-project documentation.

Remove the configure link from the .info; it doesn't go to your module.

disable_js_uninstall() is overkill. Either track all of your variables, or wildcard delete them. However, wildcard
deletion is usually frowned upon because of namespace issues, particular when a module gets sub-moduled. Personally, think about
tracking all of the settings in a single variable.

You more than likely need to implement hook_themes_disabled() to zap the settings for a particular module. When you reinstall
an uninstalled theme, it should be a clean slate. Same for hook_modules_disabled().

Settings forms don't normally need explicit submit handlers for setting variables, but to be honest I can't recall if theme
settings behave this way since theme settings are a little different.

If this really is just for themes, can't you store the settings in the theme settings and not variables? This would solve most of the
setting issues.

Your hook_js_alter will scan the filesystem every non-cached page request. This is expensive. Long term think of a
better way to handle this.

Is there a way to disable theme JS? Would be handy for some sites that use base-themes.

I would add a way to disable the functionality via a $conf[] variable in settings.php as a precaution against
leaving a site hopelessly broken as a result of removing JS. Yes, it shouldn't happen, but it could with some poorly designed themes
that assume working JS.

I would add a permission for being able to access these settings. This is a borderline security problem (someone with theme access could
disable functionality from a module).

The starred (*) issues are application blockers. See the Project Application Checklist 5.2 and 5.3
for more info. Otherwise, the checklist looks good, and none of the other points I mention are major API issues.

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.