EU Cookie Compliance

welcome,

You are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

You have to remove the LICENSE file, it will be added automatically later, also all code hosted on drupal.org is under GPL automatically.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow tips for a great project page. Also make sure your README.txt follows the guidelines for in-project documentation.

I've seen you were already working on the automated review, just ask if you got problems there
(http://ventral.org/pareview/httpgitdrupalorgsandboxmarcinpajdzik1538032git)

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

I can confirm that the automated PAReview tool does not return any errors/warnings.

Reading through the code, it all seems to check out.

One recommendation that immediately springs to mind would be to use the '#format' key on the 'eu_cookie_compliance_popup_info' and 'eu_cookie_compliance_popup_agreed' keys on the admin form. Your install file creates these variables and puts HTML in them, so doing this would allow end users to select any input method available to them and therefore enter HTML via the WYSIWYG module (if available).

I would also suggest disabling the pop-up for user-1 (admin) and also for anyone logged in and using the admin pages (though they probably would have already agreed the popup before hitting the admin pages).

Like the way the user is presented with the acceptance though... nicely done.

Corrected repository branch

The name "EU cookie compliance" implicates that the module can be used EU-wide. This appears not to be the case, as it targets UK law. Wouldn't a better name be "UK EU cookie compliance"?

In addition, the site that demos the module sets 6 cookies before the user gives consent. As such, it is not compliant with UK law as far as I understand it, and should perhaps be renamed to something else.

Marcin,Just wanted to say thanks for getting the ball rolling on this for Drupal 6. It would be great if the cookies could be disabled until a user accepts via the popup. However, I understand how busy everyone is (everyone's probably got their heads down in spreadsheets making cookie audits for their sites :-) ). I agree that this at least shows that the website owner is working towards compliance.

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

FILE: ...tes/all/modules/pareview_temp/test_candidate/js/eu_cookie_compliance.js
--------------------------------------------------------------------------------
FOUND 8 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
 29 | ERROR | There should be no white space after an opening "{"
 29 | ERROR | There should be no white space before a closing "}"
 36 | ERROR | There should be no white space after an opening "{"
 36 | ERROR | There should be no white space before a closing "}"
 72 | ERROR | There should be no white space after an opening "{"
 72 | ERROR | There should be no white space before a closing "}"
 72 | ERROR | There should be no white space after an opening "{"
 72 | ERROR | There should be no white space before a closing "}"
--------------------------------------------------------------------------------

The code looks clean an nice to me, I would just recommand modifying the following:

Grammar in project application:
do not use any => does not use
is not supported for Drupal 6 => is not supported by Drupal 6

Project page:
I recommand revising this phrase:

From that date, if you are not compliant or visibly working towards compliance, you run the risk of enforcement action, which can include a fine of up to half a million pounds for a serious breach.

bottom of website => bottom of the website

Generally it would be advisable for rechecking grammar and phrasology...

Code:

1) I recommand putting the following code into theme functions or template files (.tpl):
'value' => '<h2>We use cookies on this site to enhance your user experience</h2><p>By clicking any link on this page you are giving your consent for us to set cookies.</p>',
'value' => '<h2>Thank you for accepting cookies</h2><p>You can now hide this message or find out more about cookies.</p>',

2) Pass text to t() function:

      <button type="button" class="hide-popup-button">Hide this message</button>
      <button type="button" class="find-more-button" >More information on cookies</button>
      <button type="button" class="agree-button">Yes, I agree</button>
      <button type="button" class="find-more-button">No, I want to find out more</button>

Adding link to a review of other project.

manual review:

• eu_cookie_compliance_get_popup_content(): instead of using filter_xss_admin() you should use check_markup() to sanitize the message, as you also save the text format, right?

But otherwise this looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

One more thing: The available variables($message and $link) should be listed in the template files and

 * agree-button      - agree to setting cookies
 * hide-popup-button - destroy the pop-up
 * find-more-button  - link to information page

is same in the two tpl files.

Apart from this it is RTBC for me too. I have tested and it works well, great module. The d5 backport is done and can be tested soon. Thx Marcin!

Thanks for your contribution, and welcome to the community of project contributors on drupal.org!

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process and for your help in the application queue. 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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

Hello,

Thank you for a nice and useful module.

I have tried this module on my website.

It is working OK when it is set to the default value. However, we I set the message to pop up at the top of the website, it is not completely working.

When I click "Yes I agree" the pup up "Hide this message" option doesn't appear. When I go the a different page on the website then "Hide this message" button appears.

I was wondering if there will be any solution for this.

Thanks

@RezanShilan
this application issue is finished, please create a bug or support issue in the modules issue queue