Cannot save Skinr settings when overlay module is enabled

I can confirm this. It was working just fine last weekend (I think - though I may have had a slightly older version), but I updated today to check this out and I'm having the same problem.

This pretty much halts theme development w/Skinr for D7. Marking critical.

Works fine for me with D7 alpha3 and the CVS version of skinr.
Could you try updating D7?

This is a because of a Drupal bug with overlay.

#658720: Clean up overlay_close_dialog() and related code

I thought the above issue could cause the problem, but it's bigger #754578: Overlay adds '&render=overlay' twice to paths

Setting the status to postponed so we can have this off the active radar until it's fixed in Drupal.

Changing description to be more descriptive.

I tested this and can confirm the problem, but I also see two render=overlay's in other form actions and they do save correctly, so I'm not sure it's the problem (or the problem is in Skinr and not core), see http://drupal.org/node/754578#comment-3480832. I'm continuing to debug.

So, when the overlay is used the form_state['values'] in skinr_ui_form_submit() is:

[values] => Array
        (
            [module] => block
            [sid] => system-management
            [submit] => Save
            [form_build_id] => form-fV18nalG07k9VaL_23mytrNeUQgx4FQMgpXFWwHFCZg
            [form_token] => bLKbceF4J-72LukWMt_TgMcC0i_ZYsHBqAUBxq6VXwk
            [form_id] => skinr_ui_form
            [op] => Save
        )

You can see there that there is no skinr_settings - my submitted CSS classes etc.

But when the overlay is not used the values correctly pick up the skinr_settings element:

  [values] => Array
        (
            [module] => block
            [sid] => system-management
            [submit] => Save
            [form_build_id] => form-FeGI1icwmfypcCfo5MGWIQwSr9oMVoJbTx6hJYL6lZU
            [form_token] => bLKbceF4J-72LukWMt_TgMcC0i_ZYsHBqAUBxq6VXwk
            [form_id] => skinr_ui_form
            [skinr_settings] => Array
                (
                    [block_group] => Array
                        (
                            [bartik] => Array
                                (
                                    [_additional] => fishint
                                    [_template] => 
                                )

                            [seven] => Array
                                (
                                    [_additional] => 
                                    [_template] => 
                                )

                        )

                )

            [op] => Save
        )

So, need to dig into Skinr's form_alter and why the overlay is involved.

using the latest drupal 7 head and skinr code i am able to save skinr settings via the overlay...

setting this to needs review so others can test...

Really? This wasn't working for me over the weekend, which was the last time I tried it. Are you sure?

I just checked out drupal 7 head a few minutes ago and updated skinr via our svn the other day... its working fine for me right now.

Interesting. The most up to date version of 7.x for Skinr is in HEAD right now, not SVN. Fishy.

Odd, I'm using Skinr HEAD from d.o CVS and Drupal 7 from CVS and it's still not working to save via using the overlay.

// $Id: skinr.module,v 1.26 2010/06/02 13:16:58 moonray Exp $ Is right?

I was actually using an extremely old version of skinr lol... oops.. however after updating to skinr head, updating the db and clearing the cache this still works for me....

EDIT: i just tried again on a fresh install of d7 and its not working!! :( somehow this was working in the version of skinr i had which was ancient.. (// $Id: skinr.module,v 1.18.2.2 2010/02/01 05:50:14 jgirlygirl Exp $)

// $Id: skinr.module,v 1.26 2010/06/02 13:16:58 moonray Exp $

Yes, that's right. Thanks for trying to help with this @coltrane. ;)

I just tried, again, on a fresh install with Skinr and Drupal HEAD and it's still not working. Not sure what was happening on @nomonstersinme's system, but after doing a reinstall, it's not working for her either. It seems much more likely now that this is a Skinr problem as opposed to a core problem, but what do I know :P

A potentially lengthy approach would be to look back through the commits to Skinr till it was working. It's so odd that overlay is involved in the problem. :/

Yeah, you're not kidding. I attempted to diff the old vs. the new, but it was SOOO old. There wasn't even a Skinr UI module. Amanda is going to post the old code, so we can refer to it if need be.

This is the version of the module that i was using where saving from the overlay worked.

I'm trying to look at the differences between the code that i was using before and the latest. In skinr_form_submit its calling skinr_fetch_data and the skinr_ui_form_submit is looking for skinr_fetch_config. after this it doesn't make sense to me but perhaps there is something that happened when that was changed from skinr_data to skinr_config that is causing this? just throwing that out there..

Debugging this issue in JS, at the point of submission it tries to post to the following URL:
http://drupal7/admin/appearance/skinr/edit/nojs/block/menu-devel/configure?destination=node&render=overlay&render=overlay

It returns the error:
Failed to load source for: http://drupal7/admin/appearance/skinr/edit/nojs/block/menu-devel/configure?destination=node&render=overlay&render=overlay

One strange thing that I find is that even though $form_state['input']['skinr_settings'] exists, $form_state['values']['skinr_settings'] doesn't exist when overlay module is enabled. This is the case at the validation level, already.

Any insight as to what's going on here would be appreciated.

There is no patch here to review. Changing status.

I have no idea how, or why, but this is fully working in the latest Skinr HEAD & Drupal HEAD. :)

Skinr is confirmed to work with overlay starting DRUPAL-7-0-BETA2.
I'm posting the CVS commit of skinr that this was last tested with, in case of regression: http://drupal.org/cvs?commit=449018

Great that it's finally working. This had me stumped.