Closed (fixed)
Project:
EU Cookie Compliance (GDPR Compliance)
Version:
7.x-1.14
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
8 Sep 2014 at 12:25 UTC
Updated:
24 Jun 2015 at 13:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
ssankarsiva commentedComment #2
svenryen commentedThat's a bit strange, it's defined on line 94 as an empty variable. Thanks for the report, we'll look into this!
Comment #3
Jarlskov commentedI had the same issue. It happened when either the Background Color, or Text Color settings isn't set on the settings page. The module code on line 92 expects both of them to be set, even thought they aren't required in the form.
Comment #4
svenryen commentedI'll make sure that's fixed soon! Thanks for the report! If you have a patch that's even better ;)
Comment #5
Jarlskov commentedI can't really patch it, since I'm not sure how it's designed to react :-)
Comment #6
Jarlskov commentedOk, this is my first Drupal module patch, so maybe I'm doing something wrong. But I've tried to create a patch that sets the 2 form fields as required.
Comment #7
svenryen commentedNice work, Jarlskov. The patch should work. Though I was hoping we could do the opposite, allow the fields to be empty and the code would gracefully fail without a notice. Would you agree?
Comment #8
Jarlskov commentedWell, as I said, I'm not really sure how this was supposed to work in the first place :-)
I'm not really sure what you would expect the code to do if the fields aren't filled, but "fail without a notice" sounds like a bad idea. Either the module should be able to work without the values, or the values should be required.
If I'm allowed to leave them empty, and the module would break without telling me, I would have no chance of remedying the issue, since it's non-obvious behavior. Even for this I had to look through the code to find the issue, which shouldn't be required to use a module :-)
Comment #9
svenryen commentedYes, sorry I meant that the module should work and not give any notice when these values are missing :)
Comment #10
Jarlskov commentedHm, it seems like the issue has already been resolved with this commit http://cgit.drupalcode.org/eu-cookie-compliance/commit/?h=7.x-1.x&id=fd1..., but that's been made after the current newest tag. So it should really resolve itself with version 7.x-1.15
Comment #11
Jarlskov commentedFixed with related issue