Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
My path looks like http://localhost/core/#overlay=admin%2Fconfig%2Fdevelopment%2Fcoder%2Fup...
I would love it to look like http://localhost/core/#overlay=admin/config/development/coder/upgrade instead, for easier copy/paste of exact paths to aid in reporting bugs.
I have no idea if this is possible, but would love if it was. :) If not, maybe convert the separator to -s instead so it's not so oooglay?
Comment | File | Size | Author |
---|---|---|---|
#25 | overlayslashes.patch | 896 bytes | casey |
#23 | overlayunescape.patch | 901 bytes | casey |
#22 | overlayslashes.patch | 939 bytes | casey |
#18 | overlayunescape.patch | 901 bytes | casey |
#11 | overlayurldecode.patch | 943 bytes | casey |
Comments
Comment #1
casey CreditAttribution: casey commentedIs a '/' after a '#' valid or should it be encoded?
Comment #2
casey CreditAttribution: casey commentedAccording to http://labs.apache.org/webarch/uri/rfc/rfc3986.html#fragment: yes.
Patch
Comment #3
ksenzeeGreat! This is another obnoxious little thing about the overlay that I think is subconsciously annoying the heck out of some developers.
I would recommend assigning the return value to a variable, then returning that variable. Easier to read, especially for the poor jQuery newbies otherwise known as Drupal core developers.
I don't know if we can guarantee that "overlay" will always be the first parameter in the fragment string, especially since BBQ is in core now.
This review is powered by Dreditor.
Comment #4
casey CreditAttribution: casey commentedYou're right. Makes it a little more complicated. Do I need to explain in detail what the regexp is all about?
Comment #5
ksenzeeWhew. It certainly did get more complicated. Some explanation wouldn't be a bad idea.
Also there's some stray whitespace on the blank line.
Comment #6
casey CreditAttribution: casey commentedSmall comment added what the regexp does + minor improvement of regexp.
Assing to a variable and then return that variable seems nonsense to me. But feel free to change this.
Comment #8
ksenzeeAvoiding a long chained return value is important primarily because it promotes readability, especially for those who are new to jQuery, which (as I said a bit flippantly above) is a lot of Drupal developers. I'd personally like to see all multi-line return statements officially frowned upon in the coding standards. But in this case the long return value is especially unreadable, because there's a replace() statement with an anonymous inner function that also returns a value.
Comment #9
casey CreditAttribution: casey commentedYou can drop some braces:
string.replace() is plain old javascript, no jQuery.
Comment #11
casey CreditAttribution: casey commentedComment #15
webchickI'm trying really hard to figure out how a patch that changes some JS is causing filefields to not validate.
Comment #16
Kiphaas7 CreditAttribution: Kiphaas7 commentedwhy not just use escape()/unescape()? Granted, it has a few issues, but for the case here (prettify urls) it shouldn't be harmfull...
http://cass-hacks.com/articles/discussion/js_url_encode_decode/
Comment #17
casey CreditAttribution: casey commentedI agree we could use unescape but only on the #overlay={url} part. So we still need that regexp. Escaping everything could potentionally break other modules. I think we should not risk that.
Comment #18
casey CreditAttribution: casey commentedOk much simpler approach.
Comment #19
casey CreditAttribution: casey commentedComment #21
nvanhove CreditAttribution: nvanhove commentedTested on IE6, IE8as7, IE8, Chrome.
This patch generally works, but not completely.
Normal URLS work okay:
drupal/node#overlay=admin/people
But when there are arguments, it isn't okay:
drupal/user/1/edit?destination=admin/people%3Frender%3Doverlay
Comment #22
casey CreditAttribution: casey commentedquerystring in fragment can't be replaced jquery-bbq can't handle that.
This is the best thing we can get when using jquery-bbq (storing multiple urls in the url fragment).
Comment #23
casey CreditAttribution: casey commentedOops wrong patch.
Comment #24
ben_alman CreditAttribution: ben_alman commentedIf you're looking for a general solution to cleaning specific encoded params from a hash (generated by BBQ for example), something like this will work. Just beware that while most browsers encode params in their object-property-declaration order, that order is not guaranteed, so if there are multiple params, "overlay=" might not always follow "#" (even if overlay is the first property in the params object).
Comment #25
casey CreditAttribution: casey commentedIt contains a character set so it matches either #overlay= and &overlay=
decodeURIComponent is however very nice.
Comment #26
Anonymous (not verified) CreditAttribution: Anonymous commentedIt works for me. (Reading through it was fair too.) I can't see any holdup on it.
Comment #27
webchickOMG IT'S CHRISTMAS!!!!!
Thank you so much. :) Committed to HEAD!