Another system_theme_data() issue in v1.4 w/ PATCH [#717644]

Sorry guys, I forgot about the previous problems with system_theme_data() (#581604: skinr disable the active theme) and used that function in my patch for v1.4 (#693252: Skinr not correctly identifying inherited script/css file paths).

So here's a new patch that should fix the dblog errors and theme disabling that's happening when a site is under load (or fast refreshes).

In addition, I cached the results in a static variable to further improve performance, though it's probably not required to fix the issue.

As in the previous issue, the problem was that getting base themes in the way that looks standard--with system_theme_data()--was making a lot of database calls. With enough page hits or refreshes, that was causing a race condition described here #445052: Race Condition in system_theme_data() function. Duplicate entry error for every theme. which was causing the db errors.

So instead I'm now using _system_theme_data(), which uses cached theme information and eliminates the race condition.

Attached is the patch against Skinr v1.4.

Thanks!

Comment	File	Size	Author
#2	skinr-717644.patch	1.4 KB	jrglasgow
#2
	skinr_dblog_error.patch	995 bytes	sociotech

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Chad_Dupuis CreditAttribution: Chad_Dupuis commented 18 February 2010 at 01:00

Seems to work for me. Thanks for the quick turnaround!

I do see one new error:
array_keys() [function.array-keys]: The first argument should be an array in /modules/skinr/skinr.module on line 292.

This wasn't there before the patch as far as I can tell... From what I can see right now the error only appears to come when the admin account edits a panels page... The pages do, however, appear to load fine and all the other errors are gone.

Comment #2

jrglasgow CreditAttribution: jrglasgow commented 18 February 2010 at 04:04

File	Size
skinr-717644.patch	1.4 KB

I noticed this problem as well...
I fixed the error in the patch reported in #1

Comment #3

sociotech CreditAttribution: sociotech commented 18 February 2010 at 05:39

jrglasgow,

Thanks for the fix!

Comment #4

Jacine

she/her

New York

CreditAttribution: Jacine commented 18 February 2010 at 05:42

Status:

Active

» Postponed (maintainer needs more info)

From what I can see right now the error only appears to come when the admin account edits a panels page... The pages do, however, appear to load fine and all the other errors are gone.

Is this still the case? If so, let's fix it first.

Comment #5

Chad_Dupuis CreditAttribution: Chad_Dupuis commented 18 February 2010 at 12:45

Yes, I can reproduce the array key error everytime by clicking on the "edit panel" button on a panels page as admin:

Location http://site/admin/build/pages/edit/page-front
Referrer http://site/
Message array_keys() [function.array-keys]: The first argument should be an array in /sites/all/modules/skinr/skinr.module on line 292.
Severity error

Comment #6

jrglasgow CreditAttribution: jrglasgow commented 18 February 2010 at 13:59

Status:

Postponed (maintainer needs more info)

» Active

The error doesn't only occur when panels are being edited...
On a high load site... or on multiple page refreshes, like when you are working on testing/creating a new theme on every 4-6 page refreshes the theme doesn't load at all.

Comment #7

Chad_Dupuis CreditAttribution: Chad_Dupuis commented 18 February 2010 at 17:46

I also noticed I can no longer set skinr/fusion styles on panels pages. If I set a panel to skinr and pick a style it simply displays no style at all. Maybe I'll open a new report for this, but it seems related to the upgrade so I thought I would mention it...

Comment #8

Jacine

she/her

New York

CreditAttribution: Jacine commented 19 February 2010 at 21:28

Status:

Active

» Needs review

A fix for this has been committed to CVS for the 6.x dev version (DRUPAL-6--1). Can you guys please test it out?

Comment #9

Chad_Dupuis CreditAttribution: Chad_Dupuis commented 20 February 2010 at 22:53

So far so good. No errors, skinr settings on panels appear to save fine now. Thanks for the quick turnaround!

Comment #10

Chad_Dupuis CreditAttribution: Chad_Dupuis commented 20 February 2010 at 23:03

Actually scratch that -- sorry. Now when you save a panel as skinr it saves and everything works fine. ---But--- if you try to then save the panel as a non-skinr style (no style, rounded corners, etc.) it keep the last skinr style and simply applies the new style over top (so you get rounded corners inside of the last skinr style chosen, or no style with the last skinr style chosen...)