When clean URLs are set to "off" (on IIS), I see no overlay on node/x/edit and other places where it should appear. Instead, the full admin page is shown.
The moment clean URLs are enabled, Overlay works again.

CommentFileSizeAuthor
#10 overlay_indexphp_no_capture.patch822 bytesheine
#3 overlaycleanurl.patch776 bytescasey

Comments

casey’s picture

casey commented

Can't reproduce. Which browser/OS are you using?

heine’s picture

heine commented
Version: 7.x-dev » 7.0-alpha1

IE 8, Chrome 4, Opera 10.5 (alpha) and Firefox 3.5.7 on Windows 7 with IIS 7.5 as backend.

I've tested with both /drupal-7.0-alpha1/ and root (/) as basepath and in both cases, clean urls disabled, means no proper overlay.

When clean URLs are off, index.php ends up being used in urls (due to IIS detection code?):

edit links are: http://alpha.localhost/index.php?q=node/1/edit
instead of: http://alpha.localhost/?q=node/1/edit
casey’s picture

casey commented
Status: Active » Needs review
StatusFileSize
new overlaycleanurl.patch776 bytes

Ahh it was the index.php. This should do.

casey’s picture

casey commented
Version: 7.0-alpha1 » 7.x-dev
heine’s picture

heine commented

And so it does, but is there any reason for the backreference? Wouldn't (?:index.php)? suffice there?

casey’s picture

casey commented

Where do you see a backreference?

aspilicious’s picture

aspilicious commented

I think Heine wonna change

(Drupal.settings.basePath + "(index.php)?")

to

(?:index.php)?

casey’s picture

casey commented

That's no backreference and it also can't be changed into that.

aspilicious’s picture

aspilicious commented

Lol, ok then Heine needs to explain ;)

heine’s picture

heine commented
StatusFileSize
new overlay_indexphp_no_capture.patch822 bytes

Sorry, capture not backreference:

path = path.replace(new RegExp(Drupal.settings.basePath + "(?:index.php)?"), '');

But the non capture pattern ?: is not well supported in older browsers.

casey’s picture

casey commented

???? Why do you need a non-capture modifier?

casey’s picture

casey commented
Status: Needs review » Reviewed & tested by the community

(17:03:58) caseyn: Heine: could you explain to me why you are doing what you are doing?
(17:04:43) Heine: 1. we don't need to capture so 2. non capture might have preference with the JS folk, if not I care not
(17:05:58) caseyn: Heine: ah ok

mfer’s picture

mfer commented

I ran some tests and the non-capture is ever so slightly faster. Since we aren't using the back reference we might as well keep the ?:.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review

I can't tell from mfer's comment whether this still needs work or not. Splitting the difference at 'needs review' :)

casey’s picture

casey commented

Me neither... mfer?

bgm’s picture

bgm commented

I have run into this issue, but the patch in #10 did not solve the problem for me (I did many refresh/cache flush to make sure that the patched JS was loaded in the browser).

casey’s picture

casey commented
Status: Needs review » Reviewed & tested by the community

@bgm According to @Heine in #5 and me the patch makes it work. What browser/OS are you using?

Let's commit patch in #10. It's at least an improvement.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Sounds good. Committed to HEAD.

If this is still an issue, please post back with steps to reproduce.

Status: Fixed » Closed (fixed)

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