Introduction

Popup forms is an API module that provides an easy way for developers to display any Drupal form in a popup (through jQuery UI dialog).

How to use

Popup forms module requires jquery.postmessage plugin. Download it here https://github.com/cowboy/jquery-postmessage and place into libraries directory so that plugin will be at sites/all/libraries/jquery.postmessage/jquery.ba-postmessage.min.js

  1. Implement hook_popup_forms_data() in a custom module:
    function mymodule_popup_forms_data {
  return array(
    'test_form_id' => array(
      'params' => array('param1' => 'def_value1', 'param2' => 'def_value2'),
      // array with buttons names (#name FAPI key), that closes the popup
      'close_buttons' => array('delete', 'cancel'),
      // file needs to be included
      'file' => drupal_get_path('module', 'my_module') . '/my_module.pages.inc',
      // same use as in hook_menu.
      'access callback' => 'user_access',
      'access arguments' => array('param1' => 'def_value2', 'param3' => 'def_value3'),
      'title' => t('My form'),
    ),
  );
}
  2. Call popup_forms_parent_js() in your page callback (from where you want to show the popup) to add the necessary JS and CSS files.
  3. Create a behavior that opens popup form
    Drupal.behaviors.myBehavior = function(context) {
  $('a.my-link', context).click(function(e) {
    popupFormsFormShow(
      form_id, // Your form_id, declared in hook_popup_forms_data()
      callback, // JS callback (see below)
      form_data, // Keyed form parameters, declared in hook_popup_forms_data()
      dialog_options // JQuery UI dialog options
    );
  });
}
  4. Receive form execution result in your callback. First param of callback function is $form_state values, fetched after form submit.

The difference from other modules

There are plenty of modules that display something in popups, the most popular of them is Popup (for Drupal 7).
The main difference from this kind of modules is that Popup forms does one thing (and does it well): it opens a form in a popup.

  • The module uses jQuery UI dialog.
  • Works well with multi-step forms. Just don't add "Next" button id to 'close_buttons' array to keep popup open.
  • Can open https forms from non-secure page and vice-versa.
  • Loads form in an iFrame (no problems with form validation or AJAX-driven forms).
  • Gives you an opportunity to pass various arguments to the form callback.
  • Provides access to the $form_state from Javascript after the form is submitted successfully.

Links

Reviews of other projects

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

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

codeyourdream’s picture

codeyourdream commented
Status: Needs work » Needs review

Yep, it can not parse the following styles:

.popup-forms-dialog-wrapper {
  filter: progid:DXImageTransform.Microsoft.Alpha(opacity=0);
  opacity: 0 !important;
}

.popup-forms-dialog-wrapper.visible {
  filter: progid:DXImageTransform.Microsoft.Alpha(opacity=1);
  opacity: 1 !important;
}

Actually this is not an error.

Seppe Magiels’s picture

Seppe Magiels commented

Hi codeyourdream,

To solve the issue above try replacing it by the code below. This way it will be viewed as a string and might be accepted.

-ms-filter:"progid:DXImageTransform.Microsoft.Alpha(Opacity=50)";     /*Best for Internet Explorer 8 */
filter: alpha(opacity=50);    /*Internet Explorer 5, 6, 7, 8 */

More info on this here.

While reading filter: progid:DXImageTransform.Microsoft.Alpha(opacity=1); I noticed that you wrote opacity=1. It should be opacity=100, I made the mistake a couple of times myself, it's a sneaky one . ;)

Greetings!

Rasmus Steen’s picture

Rasmus Steen commented

I tested your module to see how it works with node add forms and I have four comments

  1. On long forms the top of the form got "cut off" and couldn't get accessed using scrolling
  2. I had to look through your source code to find out how this module is ment to work - maybe documentation could explain this better
  3. On successfull submission and after creation of the node I wasn't sure how I would close the form
  4. For some strange reason the title value of the node add form wasn't saved. The body however was saved. Was I doing something wriong?

Pastebins:
mymodule.module
http://pastebin.com/qB3Urgsa

mymodule.js
http://pastebin.com/vizGJhRH

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work
Issue tags: -PAreview: review bonus

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

Manual Review of the 7.x-1.x branch:

  1. hook_popup_forms_data(): Hooks that are provided by a module should be documented in MODULENAME.api.php, see http://drupal.org/node/161085#api_php
  2. popup_forms_theme_registry_alter(): Why do you need to do this? Please add a comment.
  3. popup_forms_form_submit_handler(): Not sure how this could be exploited here, but printing $_GET variables directly rings all alarm bells in my head. You need to sanitize all user provided input before printing, see http://drupal.org/node/28984 . It is recommended to pass down variables with Drupal.settings from PHP to JS and to put your JS into a dedicated file.
  4. "$GLOBALS['popup_forms_page_template']": all custom defined global variables of a module need to be prefixed with an underscore, see http://drupal.org/coding-standards#naming
  5. "module_invoke_all('popup_forms_child_loading');": another hook that should be documented in MODULENAME.api.php.
  6. popup_forms_process_html(): why do you need that? Why do you set variables to NULL? Please add a comment.

The potential security issue is a blocker, otherwise this looks almost ready. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

PA robot’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

codeyourdream’s picture

codeyourdream commented
Status: Closed (won't fix) » Needs review

All issues are fixed, thanks for good reviews.
Now there is no need in hooks popup_forms_theme_registry_alter() and popup_forms_process_html(), because module uses delivery callback instead.

3. popup_forms_form_submit_handler(): Not sure how this could be exploited here, but printing $_GET variables directly rings all alarm bells in my head. You need to sanitize all user provided input before printing, see http://drupal.org/node/28984 ...

Before printing these variables, drupal_json_encode() is called, which escapes <, >, ' and ". So there is no security issue.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Title: Popup forms » [D7] Popup forms
Status: Needs review » Needs work

Your README seem to have more up to date information that the project page?

Why do you have a note about D6 security when this appears to be a D7-only module?

I'm not sure why you have a new global variable $_popup_forms_ajax_response_values - is it possible to refactor this?

You have a typo in popup_forms_render_popup_form() $agument should be $argument.

$form_state['build_info']['args'] = $args; has arguments coming directly from the $_GET string, is this safe? You should probably use check_plain or something similar.

There are also some errors reported via ventral.org: http://ventral.org/pareview/httpgitdrupalorgsandboxcodeyourdream1902778git. Especially the first one (using $form_state['input']) is a security concern.

Setting to needs work for security questions, otherwise this is RTBC from me.

----
Top Shelf Modules - Enterprise modules from the community for the community.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Issue summary: View changes

Exclude info about 6.x branch, because it has been deleted from git

codeyourdream’s picture

codeyourdream commented
Issue summary: View changes

Update usage info

codeyourdream’s picture

codeyourdream commented
Issue summary: View changes

Changesome text formatting

codeyourdream’s picture

codeyourdream commented
Issue summary: View changes

Change example of file pointing

codeyourdream’s picture

codeyourdream commented
Status: Needs work » Needs review

Thanks kscheirer,
all issues are fixed, except this

$form_state['build_info']['args'] = $args; has arguments coming directly from the $_GET string, is this safe? You should probably use check_plain or something similar.

These arguments aren't printed to output, they are just passed to the form. It is the way the drupal_get_form() works in case when it acts as a page callback.

vincenzo gambino’s picture

vincenzo gambino commented
Status: Needs review » Needs work

There is still some formatting issue reported by Ventral
http://ventral.org/pareview/httpgitdrupalorgsandboxcodeyourdream1902778git


FILE: /var/www/drupal-7-pareview/pareview_temp/css/popup_forms.child.css
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 24 | ERROR | Each style definition must be on a line by itself
 24 | ERROR | Expected 1 space after colon in style definition; 0 found
 29 | ERROR | Each style definition must be on a line by itself
 29 | ERROR | Expected 1 space after colon in style definition; 0 found
--------------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/popup_forms.api.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 50 | ERROR | Function comment short description must be on a single line
--------------------------------------------------------------------------------


FILE: /var/www/drupal-7-pareview/pareview_temp/popup_forms.module
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 132 | ERROR | Function comment short description must be on a single line
--------------------------------------------------------------------------------

Please fix it.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Needs review

Minor coding standard issues are not application blockers, please do a real manual review.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Status: Needs review » Reviewed & tested by the community

Thanks for the updates, you've addressed my security concerns. Cleaning up those minor style issues would be nice, but definitely not a blocker.

----
Top Shelf Modules - Enterprise modules from the community for the community.

stborchert’s picture

stborchert
he/him
Primary language German
commented
Status: Reviewed & tested by the community » Needs work

Sorry for resetting the status, but:

PAReview: Individual user account
It seems you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please change your account information and enter your realname.

Just a personal note: I didn't see the point why (simply spoken) extracting a feature of existing modules into a new module would not be duplication of functionality ...

codeyourdream’s picture

codeyourdream commented
Status: Needs work » Needs review

Thank you, corrected the personal information.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Status: Needs review » Fixed

Thanks for your contribution, codeyourdream!

I updated your account to let you 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 get 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.

----
Top Shelf Modules - Enterprise modules from the community for the community.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Change access arguments syntax

dahousecat’s picture

dahousecat commented

I could really do with some sample code showing how to put an add node form in a popup using this module. Could anyone add some here?

I saw Rasmus added some on pastebin above but it has since been removed.

codeyourdream’s picture

codeyourdream commented
Issue summary: View changes