Overlay adds render=overlay twice to form action, making it not submit properly.
If I remove (through firefox) both render=overlay instances from the form action url, the form actually submits (but probably doesn't properly retain the overlay.

I tried playing with the lines 103-106 of overlay-child.js to reduce the number of render=overlay instances to 1, but then it does some weird drupal_goto to http://sitename.com/node?render=overlay which gets me some weird results. And, again, the form doesn't submit.

if (action == undefined || (action.indexOf('http') != 0 && action.indexOf('https') != 0)) {
  action += (action.indexOf('?') > -1 ? '&' : '?') + 'render=overlay';
  $(this).attr('action', action);
}

Perhaps someone more knowledgeable in overlay module can figure this out.

CommentFileSizeAuthor
#14 cart-modules.txt4 KBdoana
#12 multiple_overlays.png34.65 KBpancho
#7 754578-7.patch887 bytesjpmckinney

Comments

moonray’s picture

moonray commented

btw, my tests are done using skinr (which works perfectly without overlay module enabled).

jacine’s picture

jacine
she/her
Location New York
commented

subscribe.

casey’s picture

casey commented

All forms? Could you describe what you are doing, it so I can reproduce? Which browser/OS are you using?

aspilicious’s picture

aspilicious commented

In safari you can SEE the overlay loading twice, I can try to make a screencast somewhere this week.

moonray’s picture

moonray commented

Steps to reproduce:

1. install drupal
2. enable skinr module (and make sure overlay is enabled)
3. Go to the front page (root of the site) and click on the cog next to "Management" block. Select "Edit skin" (which should bring you to http://sitename.com/node#overlay=admin/appearance/skinr/edit/block/syste... )
4. Look at the "action" attribute of the form; you'll see render=overlay twice.
5. Submit doesn't even process overlay's overlay_form_submit() function.

moonray’s picture

moonray commented

Ah, and browsers tested: Safari and FF3 on Mac OSX.

jpmckinney’s picture

jpmckinney commented
Status: Active » Needs review
StatusFileSize
new 754578-7.patch887 bytes

I do a simple indexOf() to check if render=overlay is present, and if not, I allow it to be added.

I can do a more robust match() to check for (&|?)render=overlay$, but I don't think that's necessary.

moonray’s picture

moonray commented
Status: Needs review » Needs work

The patch fixes the first part of the problem: reducing the number of render=overlay to one.
But forms still won't submit properly.

A notice gets briefly flashed on submit. See #658720: Clean up overlay_close_dialog() and related code for that. But that patch doesn't fix the form not submitting, either.

So... back to not knowing why it's not working.

jpmckinney’s picture

jpmckinney commented
Title: Overlay adds render=overlay twice to form action, making it not submit » Overlay adds render=overlay twice to form action
Priority: Critical » Normal

If I do not have the skinr module enabled, I can't reproduce this with any form. I took a look at the skinr module, and although I didn't see anything wrong, it is doing complicated stuff so I could have missed it. If no one else can reproduce this error without enabling the skinr module, there's a good chance there's something wrong with the module. I'm downgrading to normal, as the only reproducible bug (Overlay adds render=overlay twice to form action) is not critical.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs work » Needs review

I looked into this and I think what's causing the double render=overlay is that it is being added both server-side (via the normal method when Drupal builds up a form) and then again client-side. The reason it gets added server side is that the Overlay module tries to remove it from the URL in http://api.drupal.org/api/function/overlay_init/7:

      // Unset the render parameter to avoid it being included in URLs on the page.
      unset($_GET['render']);

However, that doesn't really work in all cases. For example, http://api.drupal.org/api/function/request_uri/7 (which is where the form action comes from) gets its info from $_SERVER directly, so unsetting something in $_GET doesn't affect it, and that's how the server winds up putting render=overlay into the form anyway.

Setting this back to needs review, although it's not clear what the correct fix is - it would be nice if we could handle this server-side completely (and correctly.... rather than due to a bug like it is now!) and then just get rid of that JavaScript parsing altogether, but I don't know enough about the overlay workflow to know if that would break certain cases.

I also tested this out with Skinr and cannot reproduce any problem with the form not getting submitted. I added a drupal_set_message() to the Skinr submit handler to verify that it was getting called, and it was. Maybe this part of the issue fixed itself somewhere along the way?

coltrane’s picture

coltrane
he/him
commented

I can replicate the problem with Skinr (not getting settings saved) but I should try David's test to make sure the submit handler is called.

I also see two render=overlay in the form action of the Edit menu with overlay enabled. To replicate: install Drupal, keep overlay enabled, go to homepage, in the left side Management block click the gear wheel and choose "Edit menu". In the action you'll see "[..]&render=overlay&render=overlay" and an edited description is saved correctly. Because two render=overlay in another action does work, I'm suspecting an error in Skinr's case.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Title: Overlay adds render=overlay twice to form action » Overlay adds '&render=overlay' twice to paths
StatusFileSize
new multiple_overlays.png34.65 KB

This can also be observed in the dblog details.
If a watchdog event is thrown on an overlay form and you submit again and again, the overlays pile up in the logged paths (see attached screenshot).

moonray’s picture

moonray commented

Just for record's sake: the Skinr issue I described above somehow got fixed with the last beta before D7 final.
So the only issue here is now the multiple render=overlay duplication.

doana’s picture

doana commented
StatusFileSize
new cart-modules.txt4 KB

I'm experiencing this issue as well on Drupal 7.10. However, my site does not use the skinr module. Just as described above the form action lists &render=overlay twice. This occurs on every form in the overlay.

For example the form on the performance page is defined as follows:

<form id="system-performance-settings" class="overlay-processed" accept-charset="UTF-8" method="post" action="/ccs/checkout/arts/admin/config/development/performance?render=overlay&render=overlay">

Since I'm unsure what's causing this I'll attach a list of the enabled modules on my site (csv format). For the most part this site is fairly straight forward, with the exception of the commerce module.

doana’s picture

doana commented

Hi all,

This issue has been resolved for me. This site was an upgrade from Drupal 7.8, and then later 7.9 to 7.10. Each time I did this upgrade, I moved our slightly customized .htaccess file along with it. At some point, something in the .htaccess file must have changed. Restoring the .htaccess file to its default 7.10 version and then re-applying our modifications has solved the issue for me.

Thanks!

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.