New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoding all URLs #87

Closed
neocotic opened this Issue Mar 14, 2012 · 10 comments

Comments

Projects
None yet
2 participants
@neocotic
Member

neocotic commented Mar 14, 2012

Copying the URL (for example)

http://sourceforge.net/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/

as Markdown results in %20 being replaced with actual spaces rather than the percent-encodings, rendering the link broken.

Ticket: #2

After further testing it appears that even {url} replicates this. I'll look in to this ASAP.

@ghost ghost assigned neocotic Mar 14, 2012

@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

This appears to be an issue with the library used to dissect the URL decoding everything. One solution would be to use the original URL instead of the parsed URL for the {url} and other similar variables. However, should variables derived from the URL (e.g. path, segments, directory) be in their original or decoded form? If the latter, then I'll have to either find a replacement library that allows this or modify the existing one, which wouldn't be a problem but I just want to make sure I get it right.

Member

neocotic commented Mar 15, 2012

This appears to be an issue with the library used to dissect the URL decoding everything. One solution would be to use the original URL instead of the parsed URL for the {url} and other similar variables. However, should variables derived from the URL (e.g. path, segments, directory) be in their original or decoded form? If the latter, then I'll have to either find a replacement library that allows this or modify the existing one, which wouldn't be a problem but I just want to make sure I get it right.

@slhck

This comment has been minimized.

Show comment
Hide comment
@slhck

slhck Mar 15, 2012

I'd say they should be in their original form, because everything else would break pasting links somewhere. Optionally, you could maybe add a checkbox to enable or disable URL decoding for every template.

slhck commented Mar 15, 2012

I'd say they should be in their original form, because everything else would break pasting links somewhere. Optionally, you could maybe add a checkbox to enable or disable URL decoding for every template.

@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

I've already identified a quick and easy fix that will make so that nothing is decoded;

jquery.url.js:32

// var str = decodeURI( url ),
   var str = url,

So for the original example the following will variables will be derived;

directory = "/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"
path = "/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"
relative = "/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"
segments = ["projects", "octave", "files", "Octave%20MacOSX%20Binary", "2011-04-21%20binary%20of%20Octave%203.4.0"]
url = "http://sourceforge.net/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"

You can still make these values prettier using the decode operation. For example;

<!-- Input -->
{#decode}{path}{/decode}
<!-- Output -->
/projects/octave/files/Octave MacOSX Binary/2011-04-21 binary of Octave 3.4.0/

However, if you only really want the {url} variable to be pretty then that is also possible by primarily using the tab URL instead of the parsed URL within the buildStandardData method (see background.coffee:707).

Member

neocotic commented Mar 15, 2012

I've already identified a quick and easy fix that will make so that nothing is decoded;

jquery.url.js:32

// var str = decodeURI( url ),
   var str = url,

So for the original example the following will variables will be derived;

directory = "/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"
path = "/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"
relative = "/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"
segments = ["projects", "octave", "files", "Octave%20MacOSX%20Binary", "2011-04-21%20binary%20of%20Octave%203.4.0"]
url = "http://sourceforge.net/projects/octave/files/Octave%20MacOSX%20Binary/2011-04-21%20binary%20of%20Octave%203.4.0/"

You can still make these values prettier using the decode operation. For example;

<!-- Input -->
{#decode}{path}{/decode}
<!-- Output -->
/projects/octave/files/Octave MacOSX Binary/2011-04-21 binary of Octave 3.4.0/

However, if you only really want the {url} variable to be pretty then that is also possible by primarily using the tab URL instead of the parsed URL within the buildStandardData method (see background.coffee:707).

@slhck

This comment has been minimized.

Show comment
Hide comment
@slhck

slhck Mar 15, 2012

Okay, perfect, that makes sense :)

Thanks for checking that out so quickly.

slhck commented Mar 15, 2012

Okay, perfect, that makes sense :)

Thanks for checking that out so quickly.

@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

@slhck I think we're in agreement as this is the behaviour I expected but didn't test. I'll make the "decode nout" change now and consider adding the option later. I would like to get this fix in today as I really see this as a key bug.

Member

neocotic commented Mar 15, 2012

@slhck I think we're in agreement as this is the behaviour I expected but didn't test. I'll make the "decode nout" change now and consider adding the option later. I would like to get this fix in today as I really see this as a key bug.

@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

Thanks for using the new feedback system and congrats on being the first! :)

Member

neocotic commented Mar 15, 2012

Thanks for using the new feedback system and congrats on being the first! :)

neocotic added a commit that referenced this issue Mar 15, 2012

#87 ensuring bespoke jQuery-URL-Parser plugin is minified pre-distrib…
…ution (file name will be updated once it has been renamed)
@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

This is working fine for me so I'll include this in today's 1.0.6 release. I still want to rename the file (remove .min from its name) so the release will have to wait until this is done. Unfortunately, c9 isn't letting me use the git command today so I'll have to wait till after work to do this.

Member

neocotic commented Mar 15, 2012

This is working fine for me so I'll include this in today's 1.0.6 release. I still want to rename the file (remove .min from its name) so the release will have to wait until this is done. Unfortunately, c9 isn't letting me use the git command today so I'll have to wait till after work to do this.

@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

It appears c9.io is working again. The file has been renamed so I'm now going to release 1.0.6.

Member

neocotic commented Mar 15, 2012

It appears c9.io is working again. The file has been renamed so I'm now going to release 1.0.6.

@neocotic neocotic closed this Mar 15, 2012

@slhck

This comment has been minimized.

Show comment
Hide comment
@slhck

slhck Mar 15, 2012

Works as expected now — can't comment on the UserVoice thing, you've probably deleted it?

Thanks for the quick fix. Love the extension.

slhck commented Mar 15, 2012

Works as expected now — can't comment on the UserVoice thing, you've probably deleted it?

Thanks for the quick fix. Love the extension.

@neocotic

This comment has been minimized.

Show comment
Hide comment
@neocotic

neocotic Mar 15, 2012

Member

That's great and thanks for the feedback.

Not sure why it wouldn't let you comment on the ticket as this is the first time I've used it. The ticket is still open though and I've definitely not deleted but I'll close it off now since it's resolved.

https://template.uservoice.com/admin/tickets/#!search?id=2

Member

neocotic commented Mar 15, 2012

That's great and thanks for the feedback.

Not sure why it wouldn't let you comment on the ticket as this is the first time I've used it. The ticket is still open though and I've definitely not deleted but I'll close it off now since it's resolved.

https://template.uservoice.com/admin/tickets/#!search?id=2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment