After updating to 7.x.1.10 version, i noticed that every time i try to "Edit rule", i get 14 php notices for "Undefined index" in the database log. Here are some of them:

Notice: Undefined index: crid in css_injector_edit() (line 107 of /home/.../sites/all/modules/css_injector/css_injector.admin.inc).
Notice: Undefined index: crid in css_injector_edit() (line 107 of /home/.../sites/all/modules/css_injector/css_injector.admin.inc).
Notice: Undefined index: title in css_injector_edit() (line 148 of /home/.../sites/all/modules/css_injector/css_injector.admin.inc).
...

This does not happen when a rule is created or saved but only when the "Edit rule" button is clicked.

CommentFileSizeAuthor
#10 css_injector-bad_crid_protection-2160385-10.patch1.35 KBsherakama
#9 css_injector-bad_crid_protection-2160385-9.patch1.03 KBsherakama
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oresh’s picture

oresh CreditAttribution: oresh commented

Straange... Can't reproduce.
I'll try to explore more this evening.

Thanks for report!

oresh’s picture

oresh CreditAttribution: oresh commented

I made a bunch of test after updating from 1.8 to 1.10
The error you are getting is form the function that was not changed in the new release. It's related to the rule existence in the database, so when you're trying to edit it - it can't be found and gives an error.

Did you run update.php after updating? This shouldn't fix the problem, but it can be anything )

nkleopas’s picture

nkleopas CreditAttribution: nkleopas commented

Did you run update.php after updating?

Yes, i did run update.php

I installed the module on a new drupal installation and everything worked fine.

nkleopas’s picture

nkleopas CreditAttribution: nkleopas commented

I think i found what is causing the problem.

When the "Aggregate JavaScript files" is checked in the "Performance" section (admin/config/development/performance), the error occurs.

Also, the colors in the editor (syntax highlighter) are not visible and generally the editor is not working normally.

Unchecking the "Aggregate JavaScript files" fixes the problem.

oresh’s picture

oresh CreditAttribution: oresh commented

nkleopas,
hm... Thanks for your investigation. I'll probably add Ace as library, so all the files will be loaded at once - hope it will fix the issues.

Thanks for helping with the module! )

srobert72’s picture

srobert72 CreditAttribution: srobert72 commented

Same problem for me (I have "Aggregate JavaScript files" checked), what could I do to fix it ?

John Bickar’s picture

John Bickar CreditAttribution: John Bickar commented

The PHP undefined index errors do not occur with a clean install of 7.x-1.x-dev on simplytest.me, but the syntax highlighting does not work (this is with JS aggregation enabled).

I am experiencing this issue (the PHP undefined index errors) on a site that had an earlier version of css_injector (7.x-1.10) and has been upgraded to 7.x-1.x-dev (at 41bee).

(Edit: updated version numbers. I think this occurs when upgrading from 7.x-1.10 to 7.x-1.x-dev. Will test.)

John Bickar’s picture

John Bickar CreditAttribution: John Bickar commented
Version: 7.x-1.10 » 7.x-1.x-dev

Strike that. I can trigger the undefined index errors with a clean install of css_injector-7.x-1.10 and JS aggregation enabled.

It does not appear to be related to an upgrade path.

Still persists in latest -dev.

sherakama’s picture

sherakama CreditAttribution: sherakama commented
FileSize
css_injector-bad_crid_protection-2160385-9.patch1.03 KB

Somewhere it appears that an incorrect path is being creating and is causing the CRID parameter to be a non-numerical value.

eg: /admin/config/development/css-injector/edit/mode-css.js
/admin/config/development/css-injector/edit/theme-chrome.js

Here is a patch to protect against a bad crid until myself or someone else figures out where these bad requests come from.

sherakama’s picture

sherakama CreditAttribution: sherakama commented
FileSize
css_injector-bad_crid_protection-2160385-10.patch1.35 KB

Figured it out.

Looks like the ace and syntax highlighter javascript libraries do not play nice with the aggregation. Small change to their attach rules in patch.

sherakama’s picture

sherakama CreditAttribution: sherakama commented
Status: Active » Needs review
John Bickar’s picture

John Bickar CreditAttribution: John Bickar commented
Status: Needs review » Reviewed & tested by the community

That patch looks like a sensible approach, and it resolves the undefined index errors. RTBC.

Robin van Emden’s picture

Robin van Emden CreditAttribution: Robin van Emden commented

Agreed, tested and seems RTBC.

mariem’s picture

mariem
Primary language English
CreditAttribution: mariem commented

The patch in #10 worked for me as well, resolving the undefined index errors and enabling syntax highlighting. Thanks!

fonant’s picture

fonant CreditAttribution: fonant at Fonant Ltd commented

+1 Patch #10 fixes the problem.

oresh’s picture

oresh CreditAttribution: oresh as a volunteer commented

sherakama, thanks for the commit!
Pushed to 7.x-1.x branch. http://cgit.drupalcode.org/css_injector/commit/?id=1dd9376

oresh’s picture

oresh CreditAttribution: oresh as a volunteer commented
Status: Reviewed & tested by the community » Closed (fixed)
oresh’s picture

oresh CreditAttribution: oresh as a volunteer commented
Status: Closed (fixed) » Patch (to be ported)

not closed yet, sorry. no release yet.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Patch (to be ported) » Fixed

Closed fixed is the correct status of this patch, it's committed to this versions dev branch.

Well I agree there should be a new release, that is up to the maintainer of the module.

You can always offer to co-maintain a module in a new issue if you can spare the time to review, commit and release patches.

Status: Fixed » Closed (fixed)

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

keithn’s picture

keithn CreditAttribution: keithn as a volunteer commented

#4 worked for me.