stephenmcd / grappelli-safe

Admin skin for Mezzanine
Other
25 stars 59 forks source link

Fix popup query parameter in changelist template #58

Closed dsanders11 closed 8 years ago

dsanders11 commented 9 years ago

Same fix as this commit from Grappelli upstream

stephenmcd commented 9 years ago

What does this fix? Have you tried it?

dsanders11 commented 9 years ago

I have tried it, yea, that's how I stumbled upon it.

In an admin popup for a change list (which happens when you use the related object lookup) if there's a search box available, and you search, it has the 'N total' link below the search box like in the normal admin, which is a link to the full list of objects. At the moment since grappelli-safe has 'pop=1' instead of '_popup=1', clicking that link will strip off the '_popup=1' from the URL and cause havoc.

stephenmcd commented 9 years ago

Ok, cool - I had just grepped the code base for "pop" and couldn't see a single reference to "_popup" anywhere (but many results for "pop").

dsanders11 commented 9 years ago

Ah, gotcha. Yea, '_popup' is used for denoting the admin view as a popup so that it doesn't show the list of apps and other stuff I'm sure.

dsanders11 commented 8 years ago

Is there anything that needs to be done further with this PR that's preventing it from being merged?

stephenmcd commented 8 years ago

I think I came across other instances of "pop=1" - there's some stuff in filebrowser and mezzanine that reference it, so I wasn't sure how complete this is.

But I just now tested the single case this actually deals with and given that there's another "_popup=1" already in this file I guess it's good for now.

Thanks!