Save configuration opens a new browser window

Can you add some details such as browser and the admin theme you are using. Sounds odd and not something I have run into - this is certainly not default behavior.

So I was having this problem too. In my case, it is brought on by Domain Access with the SEO option to always link to content on its source domain turned on.

The offending code is in modules/overlay/overlay-child.js

    var action = $(this).attr('action');
    // Keep internal forms in the overlay.
    if (action == undefined || (action.indexOf('http') != 0 && action.indexOf('https') != 0)) {
      action += (action.indexOf('?') > -1 ? '&' : '?') + 'render=overlay';
      $(this).attr('action', action);
    }
    // Submit external forms into a new window.
    else {
      $(this).attr('target', '_new');
    }

There are several things wrong with this code.

First of all, checking for http or https in the url is not actually testing if the form is "internal". In my case, domain access was putting http://mydomain.com in front of all urls, even if it wasn't leaving the site. I suspect other SEO modules designed to reduce duplicate search engine content would do the same.

Secondly, a problem which was fixed on another issue, is that it needs to be careful not to pile on additional "render=overlay" values if one is already there.

Lastly, there is no "_new". This is embarrassing. If you want to use target= to open a new tab/window the correct value is "_blank". "_new" is treated just as any other named window such as "kittenmittens", in that subsequent links to target="_new" will reuse the window/tab originally named "_new"

This patch first uses a regular expression to extract the hostname portion of the url, then a couple of tests to eliminate any username/passwords or port numbers. It works across protocols and on protocol independent URLs beginning with //hostname.

I don't have git working so my patch isn't a real git patch, so who knows if it'll work. But it's free for anyone to use and adapt.

Cheers!

Thanks for starting the patch!

+      // Make sure render=overlay is not added more than once.
+      if (action.indexOf('render=overlay') == -1) {
+        action += (action.indexOf('?') > -1 ? '&' : '?') + 'render=overlay';
+        $(this).attr('action', action);
+      }

That issue is already being discussed and worked on at #754578: Overlay adds '&render=overlay' twice to paths, so no need to duplicate it here... unless I'm missing something and it's somehow required to solve this bug too.

-      $(this).attr('target', '_new');
+      $(this).attr('target', '_blank');

I don't think either of these are correct; seems to me like it should be '_parent' instead. See #667012: Remove the opening of external links in a new browser window.

I wasn't sure what do to about #754578. It is a very important fix, one that I had already implemented on my site, which is how it made it into the diff. I chose to leave it there and point out that it was addressed elsewhere.

I like _parent as well. I wasn't even thinking about what "should" happen, just how it was being implemented.

I think we should remove the other fix from this patch. It's better to keep separate issues for separate bugs so the fixes can be evaluated separately.

And yeah, the '_parent' change... I wish we had done that as part of #667012: Remove the opening of external links in a new browser window (since it was certainly the intention of that issue that the overlay not open up new browser windows anymore at all). I think I just didn't realize it was in this part of the code also.

I wasn't sure what do to about #754578. It is a very important fix, one that I had already implemented on my site

From what I remember of that issue, it didn't seem like that big of a bug - just extra stuff in the URL, but it didn't have any major side effects beyond that. However, when that issue was originally filed, it seemed like there was some thought it might have caused other problems. Do you know of specific problems that it causes?

If so, please leave a comment on that issue explaining what they are and how to reproduce - maybe the priority of that bug could be bumped up in that case.

If it helps, I didn't see this behavior until updating to Drupal 7.2.

We do need a generic function to test if a URL is external or not. I posted a patch today at #1174686-2: Overlay parent improperly opens child overlay on specific external links that adds such a function. Would that be good enough here? If not, let's at least extract the code here into a separate function that could be used elsewhere.

Having same problem, triggered same as #3, version 7.14. Glad to at least find out what's triggering this!

Given that ksenzee's patch on #1174686 is now in both D8 and D7, I think the only patch we need for this issue is the one I'm attaching.

Only concentrating on the main issue for now, that certain forms in overlays open in a new window, ignoring the 2 side issues of the "_new" target and multiple render=overlay on urls, which have their own issues.

Note also that this issue is relatively easy to reproduce if you are forcing all your forms to #https=TRUE, because that in effect makes the action an absolute url, even though in most cases it's internal. This trips up on the original check whether there is a http(s) at the beginning of the action url.

I am able to verify that the D7 version of this patch solves my instance of this problem, which was caused by a combination of this issue and forcing all forms to SSL. I haven't yet tested the D8 version posted here due to time constraints.

This would be the D7 version.

Looking at it now, I'm wondering whether it is more intuitive to invert that condition. Probably doesn't help much.

updating issue summary using the template

#11: 1082032-overlay-form-external.patch queued for re-testing.

#11 causes javascript errors - overlay-parent.js isn't included in the overlay child frame, so isExternalLink (in fact the entire Drupal.overlay object) in not defined.

We'd need a way to access this function from both the overlay parent and child - possibly it needs to exist somewhere more generic.

So, the alternatives are:

1. call the function on the parent window, where we can be more or less sure the function always exists
2. make this function more generally available and not tie it to Drupal.overlay (which is not available in the overlayChild)
3. add a copy of the function to overlayChild and call that

I'm attaching a patch that does (1). I'm not keen on (3) due to duplication, and (2) doesn't seem entirely necessary if we can guarantee that an overlayChild always has a parent window with a Drupal.overlay object (which sounds like under all reasonable circumstances would be the case).

D7 equivalent patch

I closed a similar issue, which I think is a duplicate: http://drupal.org/node/1530302

Patch in #18 seems to be working on my D7 install, I ran into this with the Domain Access module installed which made all form submissions in the overlay open in a new window, but the patch seems to correct the problem.

A wonder what other testing is required (or possible) here. Tempted to RTBC.

Patch #18 works for me.

It is possible to commit to D7 core?

Patch in #18 does not work here at same constellation: D7/domain access.

@Vincent_Jo Did you clear your cache after applying #18?

I can confirm the patch in #18 solves the problem in D7 in combination with the domain access module.

I have applied the patch in #18, and cleared my cache, but saves on overlay forms still open new windows. I'm using D7.23, Domain Access 7.x-3.10. The problem only occurs when I have " Rewrite all URLs to point to a single source" turned on under Domain Settings. When this SEO setting is turned on I get the exact same result, with or without the patch.

fix issue link

Overlay is dead to D8 #2088121: Remove Overlay.

re-posting Alan's patch form #18 so it runs tests (though it's JS)

Hi, I can confirm this comment by kenheim at #26:

The problem only occurs when I have " Rewrite all URLs to point to a single source" turned on under Domain Settings. When this SEO setting is turned on I get the exact same result, with or without the patch.

Same here.

regards
Vincent

I want to add my concern here. I have the same problem as #26 and #29. The problem only occurs when I have "Rewrite all URLs to point to a single source" turned on in Domain Access.

In RE #30 - turning off "Rewrite all URLS to point to a single source" in Domain Access configuration worked for me as well.
Thanks!

This isn't fixed when "Rewrite all URLs to point to a single source" is selected in Access Domain as stated by #26, #29 and #30.

Bump. Could this be fixed over in Domain Access? *thinking cap on*

Patch above does seem to work, at least in my use case.