How to set password with mPDF

I am attaching a patch which allows users who are using mPDF as third party tool to create a password protected pdf and is also configurable.Please give your suggestions if you have any related to this.Please review
Thank you.

Changing status.

+++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
@@ -0,0 +1,59 @@
+  $form['settings']['permission'] = array(
+      '#type' => 'fieldset',
+      '#collapsible' => TRUE,
+      '#collapsed' => TRUE,
+      '#title' => t('PDF Password Protection'),
+    );

You add a collapsed fieldset around a single field. This cluttering the form needlessly. On top of that you collapse the fieldset. What is there to hide? Don't use a fieldset here.

1. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +    $pwd = variable_get('pdf_using_mpdf_pdf_password');
   

   The variable does not use the module namespace. Only use 'print_pdf_* variable names.

2. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +    $pwd = variable_get('pdf_using_mpdf_pdf_password');
   +    if (isset($pwd) && $pwd != NULL) {
   

   Why use isset() and != NULL? Just give the variable a default value of an empty string and use if($pwd)

   Is $pwd a good descriptive variable name? $password is more explanatory.

3. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +        '#markup' => t('<p>Password : ******** is already set.</p>'),
   

   Don't include HTML inside t() unless unless there is no alternative. Place it outside the t(). It confuses translators.

   I'm not fond of the "****". "Password is already set." is clear enough.

4. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +      $form['settings']['permission']['remove_pwd'] = array(
   +        '#type' => 'checkbox',
   +        '#title' => t('Remove Password'),
   +      );
   

   Just remove the password? Perhaps also give the user the opportunity to enter a new password.

5. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +        '#description' => t('If password is not required, leave blank. Do not use space in starting and ending of password.'),
   

   Try to write positive sentences. For example: "Enter a password to protect the pdf document. Users will need to enter this password before they can read it's content."
   Second sentence is not proper English.

6. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +$form['#validate'][] = '_print_mpdf_settings_validate';
   

   My not use the default form validation function name?

7. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +      if ($form_state['values']['remove_pwd'] == '1') {
   

   I don't think you need to check for value == '1'.

8. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/pdf_using_mpdf.admin.inc
   @@ -0,0 +1,59 @@
   +        variable_set('pdf_using_mpdf_pdf_password', NULL);
   

   If the default is really NULL (see above), you can use variable_del().

9. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/print_pdf_mpdf.info
   @@ -3,3 +3,4 @@ description = "PDF generation library using mPDF."
   +configure = admin/config/user-interface/print/pdf/mpdf
   \ No newline at end of file
   

   Always end a file with an empty line. See https://www.drupal.org/coding-standards#indenting

10. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/print_pdf_mpdf.module
    @@ -26,6 +26,26 @@ function print_pdf_mpdf_pdf_tool_info() {
    +  return $items;
    +}
    +
    +
    +/**
      * Implements hook_pdf_tool_version().
    

    This may not be in coding standards, but be consistent in empty lines. I always use one between functions.

11. +++ b/print_pdf/lib_handlers/print_pdf_mpdf/print_pdf_mpdf.pages.inc
    @@ -54,6 +54,12 @@ function print_pdf_mpdf_print_pdf_generate($html, $meta, $paper_size = NULL, $pa
    +  $password = variable_get('pdf_using_mpdf_pdf_password');
    +  // Setting Password to PDF.
    +  if (isset($password) && $password != NULL) {
    +    // Print and Copy is allowed.
    +    $mpdf->SetProtection(array('print', 'copy'), $password, $password);
    +  }
       drupal_alter('print_pdf_mpdf', $mpdf, $html, $meta);
    

    It feels like this code is squeezed in. Use an empty line to separate parts of code that do different things. See it a the separations between paragraphs of text.

    Start the 'paragraph' of code with explanation instead of short sentences in between.

    You make assumptions about the use of the password (print and copy). Should this not be in the settings form?

12. +++ b/print_pdf/print_pdf.admin.inc
    @@ -128,7 +128,6 @@ function print_pdf_settings() {
         );
    -
         $form['settings']['link_text'] = array(
    

    Remove empty line?

13. +++ b/print_pdf/print_pdf.admin.inc
    @@ -156,7 +155,7 @@ function print_pdf_settings() {
    -
    +  ¶
       return system_settings_form($form);
    

    Lines should have no trailing whitespace at the end. https://www.drupal.org/coding-standards#indenting