[D7] Simple Bookmarklet

Timeout when invoking pareview.sh for http://git.drupal.org/sandbox/caiosba/2215149.git at http://pareview.sh/pareview/httpgitdrupalorgsandboxcaiosba2215149git

Do you have any third-party files committed? 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.

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.

Hello,

You should not work on master branch, see maintaining a drupal project with git.

You are including in your sources a jquery version, this is not supposed to happen this drupal is already shipped with jquery.
Your module lacks php doc and online comments, so it's quite obscure what you are doing in it.
It would be nice to put your tpl files in a separate folder.

Hi caiosba,
I appreciate your idea of project but i have few doubts.
Given video link is 404(Not Found).

Below are my reviews:-

1. By Coder Module:-

File: @file block missing (Drupal Docs) [comment_docblock_file]
severity: normalreview: i18n_10Line 26: Menu item titles and descriptions should NOT be enclosed within t(). [i18n_10]

'title' => t('Bookmarklet'),

severity: normalreview: i18n_10Line 27: Menu item titles and descriptions should NOT be enclosed within t(). [i18n_10]

'description' => t('Administer bookmarklet settings.'),

severity: normalreview: i18n_10Line 33: Menu item titles and descriptions should NOT be enclosed within t(). [i18n_10]

'title' => t('Bookmarklet'),

severity: normalreview: i18n_10Line 39: Menu item titles and descriptions should NOT be enclosed within t(). [i18n_10]

'title' => t('Simple Bookmarklet'),

severity: criticalreview: security_3Line 346: Potential problem: drupal_set_message() only accepts filtered text, be sure all !placeholders for $variables in t() are fully sanitized using check_plain(), filter_xss() or similar. (Drupal Docs) [security_3]

drupal_set_message(t('Content created successfully.',

2. In Readme file, Column not allowed more than 80 characters. Please check with online tool.

3. In .module file, at line 217

return "javascript:(function(){window.simpleBookmarkletProtocol=(((document.location.protocol=='https:')||$force_https)?'https':'http');window.simpleBookmarkletURL='$setting_url'.replace(/^https?/, window.simpleBookmarkletProtocol);if(window.simpleBookmarklet!==undefined){simpleBookmarklet();}else{var src='$js_url?$timestamp'.replace(/^https?/, window.simpleBookmarkletProtocol);document.body.appendChild(document.createElement('script')).src=src;}})();";

isn't this possible using any custom js file.?

Changes look good to me, thanks caiosba.

What's needed to move this application forward?

Hi.
It's better to move simple_bookmarklet_admin_settings() to a separate simple_bookmarklet.admin.inc file.

So module code will be:

  $items['admin/config/content/simple_bookmarklet'] = array(
    'title' => 'Bookmarklet',
    'description' => 'Administer bookmarklet settings.',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('simple_bookmarklet_admin_settings'),
    'access arguments' => array('administer site configuration'),
    'file' => 'simple_bookmarklet.admin.inc',
  );

That is surely not an application blocker, any other problems you found? If not then the correct status would be RTBC, right?

Hi again.

1. I got this message directly in bookmarklet window when user is not logged in Drupal site.
"Notice: Undefined variable: prepopulate in simple_bookmarklet_page_alter() (line 388 of /var/www/hosts/drupal1/sites/all/modules/simple_bookmarklet/simple_bookmarklet.module)."
See screenshot.
It would be better to check existing of $_GET['simple_bookmarklet_prepopulate'] and fill $preopopulate by default some value.

2. Also I got are some javascript problems. I cannot open/close bookmarklet window on some sites: lenta.ru - cannot close, gmail.com - cannot open. Tested in FF 29.0 and Chrome 34.0.1847.137 under Linux.

Not sure I understand the process of being granted a git account here: is the module supposed to be completely bug-free to be accepted? Are all bugs going to be tracked on this single thread? What are the criteria for acceptance?