When #overlay=admin/structure/trigger is activated, it redirects to #overlay=admin/structure/trigger/node%3Frender%3Doverlay. This leads to inability to do simple Back because the previous URL is #overlay=admin/structure/trigger which redirects again.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cbrookins’s picture

Also the back button hangs after Modules > Permissions. I reproduced this in alpha2. Back button works in other circumstances.

casey’s picture

And how does it work without the overlay?

ogi’s picture

I've just tested Drupal 7 CVS again. Without overlay, there's no redirection and so there's no problem.

casey’s picture

Priority: Normal » Critical
FileSize
1.27 KB

Oww I get it. Drupal.overlay.syncChildLocation in overlay-parent.js is causing an extra redirect which also is added to the browser's history.

Patch fixes the history, but we might wanna reconsider this syncing; it costs 2 extra http requests.

casey’s picture

Status: Active » Needs review
casey’s picture

FileSize
1.29 KB

Sorry added patch without checking; forgot variable assignment.

casey’s picture

Edit: my note in#4 was nonsense. Patch is good.

ksenzee’s picture

Status: Needs review » Needs work

Thanks for the patch casey!

+++ modules/overlay/overlay-parent.js	24 Feb 2010 13:57:06 -0000
@@ -870,10 +870,16 @@
+    newLocation = newLocation.replace(/(%3F|%26)render%3Doverlay/, '');

Ahh, regex. This breaks down if you have multiple query parameters in the destination. Say the destination is admin/something?render=overlay&someparam=somevalue . Encoded, that gives us destination=admin%2Fsomething%3Frender%3Doverlay%26someparam%3Dsomevalue . Run this regex on it, and you get admin%2Fsomething%26someparam%3Dsomevalue , which decodes to admin/something&someparam=somevalue . The regex needs to preserve the question mark in the destination URL.

Since this is going to get a bit complicated, I wonder if it wouldn't be easier to use the approach you took in fragmentizeLink and decode this before applying the regex, then encode it again.

+++ modules/overlay/overlay-parent.js	24 Feb 2010 13:57:06 -0000
@@ -870,10 +870,16 @@
+    $.data(window.location, newLocation, 'redirect');    

Some trailing whitespace snuck in.

+++ modules/overlay/overlay-parent.js	24 Feb 2010 13:57:06 -0000
@@ -870,10 +870,16 @@
+    // location.replace doesn't create a history entry. location.href does.
+    // In this case, we want location.replace, as we're creating the history
+    // entry using URL fragments.

This comment appears elsewhere in overlay-parent.js - let's simplify it here with something like "Using location.replace so we don't create an extra history entry." Or something like that.

Powered by Dreditor.

casey’s picture

Status: Needs work » Needs review
FileSize
1.38 KB

- Moved regexp to fragmentizeLink() which is a better location and made it work with multiple params.
- Removed trailing whitespace.
- Simplified comment.

marcvangend’s picture

Version: 7.0-alpha1 » 7.x-dev

No activity here for a month; What is the status? Bumping this to 7.x-dev so it will show up better in the issue queues.

marcvangend’s picture

#9: overlaysyncchild.patch queued for re-testing.

sun’s picture

Priority: Critical » Normal
Status: Needs review » Reviewed & tested by the community

Looks good. However, this issue really stretches the definition of critical...

klausi’s picture

#9: overlaysyncchild.patch queued for re-testing.

klausi’s picture

FileSize
1.31 KB

Simple reroll.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.