The form api update seems to have broken this. Previously the themes where aggregated into one system variable and saved as one serialized variable. The new form api version saves each field on the form as individual variables which breaks all of the theme settings commands. Looking into creating a patch to fix the form generation code now.

CommentFileSizeAuthor
#1 themesettings.patch882 bytesccourtne

Comments

ccourtne’s picture

ccourtne commented
Status: Active » Needs review
StatusFileSize
new themesettings.patch882 bytes

Here is a patch that fixes the theme settings getting saved appropriatley. There still seems to be an issue of theme specific setting not overriding defaults which I have not figured out yet.

gtcaz’s picture

gtcaz commented

Possible related issues:

http://drupal.org/node/34170
http://drupal.org/node/34086
http://drupal.org/node/34705

gtcaz’s picture

gtcaz commented

Your patch failed for me, but I applied it by hand and it looks like theme settings are now saved. Thanks!

gtcaz’s picture

gtcaz commented

On further testing, settings that *were* previously working are not. I am now able to toggle "Display post information on" (which didn't save before) but am unable to "Toggle display" of any elements and have that setting save properly.

James Addison’s picture

James Addison commented

I too am experiencing this with the CVS version pulled as of this evening (Oct 21, 2005). I was hoping that the CVS version was close enough to 4.7.0 completion - but I guess not. ;)

dries’s picture

dries commented

I want a form API expert to review this patch, and to confirm that this is the best possible approach. Please review this carefully.

(Either way, the patch's coding style needs work.)

ccourtne’s picture

ccourtne commented

I agree with Dries that a "forms api expert" should review. This was the quick patch to get things to save like before the patch. There are four other ways this could be handled. First we could give theme settings it's own execute method and have the special handling there although that code would almost be identical to what this patch did. Second we could wrap the entire theme settings form in a fieldset with a "#tree" => true property. It didn't seem right to modify the UI to add a false grouping for the entire form. The third option would be to completely revamp the theme settings to use individual variables instead of a serialized array. Forth we could add a pseduo control to the from with creates a tree like "fieldset" but that doesn't actually generate html. Both the third and forth options seemed to big considering the impending 4.7 freeze.

Also gtcaz as I said this does not fully resolve all the theme settings issues. I am unsure if the rest of the bugs in the theme settings pages and behaviors are related to form api changes or not. Things like overriding display toggles on an individual theme seem to save properly in the database now, but don't seem to take effect. I did not get time to try and debug them further.

What "coding style" issues are there? Is it bad form on erasing the array and substituting it? Is not using some neato keyword that would remove the need for a loop?

colorado’s picture

colorado commented

I just installed from CVS and have theme irregularities, specifically:

Blocks are only showing in the bluemarine theme, see my post http://drupal.org/node/35138

Also, no other theme will work unless I un-check bluemarine and make it not available. Then other themes can be activated, but no blocks will appear in any other themes.

I looked in the database and found an error on the block table - it has NO INDEX KEY!

Could this be a problem??

ccourtne’s picture

ccourtne commented
Status: Needs review » Fixed

Looks like unconed fixed this in head. Although there is still bugs around theme specific config not getting used correctly.

chx’s picture

chx commented

Adrian did, but nevermind that.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)