( now say that title 3 times in a row, fast, without breaking your tongue :p )

Anyway, I have a 18n-enabled site, and the url() function makes it so that my URLs in are displayed like 'http://MYHOST/nl/lql' instead of 'http://MYHOST/lql'.

Nooooo! 3 extra characters! *choke*

We don't need to bother with url(), which is meant to do all kinds of tricks to make multilanguage aliases (and/or different hostnames and other things) work. We just want the short URL, no prefixes or stuff, because shorturl_init() is going to catch the unmangled short URLs anyway.
And we always want the hostname that is configured in shorturl.module; that's what it's there for. It might be different from the 'official' hostname on our site.

See the attached patch.

(I've also managed to make the language prefix go away using this:

$url = url(shorturl_shorten($original), array('absolute' => TRUE, 'path' => TRUE, 'language' => (object)array('prefix' => '')));

...but that's overly complicated, doesn't feel safe, and it ignores shorturl.module's separate hostname setting.)

CommentFileSizeAuthor
shorten.shorturl.patch605 bytesroderik

Comments

icecreamyou’s picture

icecreamyou commented
Project: Shorten URLs » Short URL

Moving to the correct project.

icecreamyou’s picture

icecreamyou commented
Project: Short URL » Shorten URLs
Priority: Normal » Minor
Status: Needs review » Needs work

Whoops, my bad, I misread the issue.

Your patch will not work for all sites. There is a lot more that goes into figuring out which base URL to use than your patch acknowledges -- see url() as an example. For starters, your patch won't work on sites using HTTPS, or (probably) that have subdomains, or (probably) that run on localhost, or (possibly) that run in a subfolder.

I'm not quite sure what the best solution here is; I'd appreciate some feedback from the Short URL maintainer on this one, especially because I don't use that module. I also have no experience with multilingual websites. Three extra characters is a much smaller concern for me than working URLs.

irakli’s picture

irakli commented

IceCreamYou is right, the patch is not a solution. Interesting problem, though, thanks for uncovering it, roderik.

I think the solution is to continue using url(), but with different arguments. I would try to play with 'alias', 'language' and 'prefix' arguments that can affect the attachment of the language prefix. Some combination of these should lead to the desired affect: http://api.drupal.org/api/function/url/6

This is a perfect candidate for writing simpletest test-cases to make sure different edge cases are properly covered, by the way.

P.S. I agree that issue belongs to the shorten project, since the file being patched is part of that project?

Thank you

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

@IceCreamYou:

I know what url() does. I've checked the whole routine and I have come up with a solution that produces the desired effect. It's posted in my first message, for reference.
(However, even though what I posted works correctly... I did make a mistake. I wanted to include the 'alias' argument too, which is conceptually better even though it does not really affect the return value. Here is what it should have said.)

$url = url(shorturl_shorten($original), array('absolute' => TRUE, 'alias' => TRUE, 'language' => (object)array('prefix' => '')));

From my experiments and looking through the code, I don't think there is another solution than this one.

But I consciously decided to scrap the call to url(), after I tested this and thought further. You know why?
* url() changes a path, so that it is guaranteed to be a 'valid URL to a Drupal page'.
* The output from the shorturl service is not a 'valid URL to a Drupal page'. It's just some random short string, that redirects somewhere else. (Therefore it should never even be a URL leading to a page on your site.)

We are talking about different concepts here. And that is the underlying reason why using url() is only possible with such convoluted arguments as above example. And why url() should not be used in this case.
url() should be used for the first thing, not for the second.

We can see that these are different concepts, because most of what url() does is invalid for generated short-urls. Think about the reasons you mentioned: "these things will not work on":

- localhost: what? You want a generated-short-url to be "http://localhost/lql"? That's not very useful :-)

- subdomains: Yes let's talk about domains. If you use two domains for each language, the reply you get from the 'shorten' page will be different depending on the interface language. If you view the page in English, your result will be "en.domain.com/lql" - and if you view the page in French, it will be "fr.domain.com/lql". Why should it be different?
(By the way, since the Short URL module only stores the URL part, not the domain, URLs with both domains will be correctly redirected. The only thing we're talking about here, is what is presented to the user, as the result of a "Shorten" action (on the /shorten page)

- sites that use HTTPS or run in a subfolder: You're right about that. But please show me a https site or a site with an extra URL-part(subfolder) that is going to be used as a 'ShortURL' :-)
(NB: it might be that some exotic person wants to have "https short urls". But if so, think that should be configured in the Short URL module, not in your url() modifier. See below.)

---

I will give you a 'counter example' why url() should not be used:
My own setup :-)

My website is http://roderik.muit.nl. This is the official hostname, as far as Drupal core is concerned, and I don't want to change that.
However I also want to use this Drupal instance as a Shorten/ShortURL service, which generates short URL for my 'twitter announcements of new posts'.
So I've:
- made g.wyz.nl another hostname in my webserver config, so it also serves pages from the same Drupal instance
- put 'g.wyz.nl' as the hostname in admin/settings/shorturl.

I'm sorry I don't have a live example for you, because I shortened 0 URLs so far, and it will be a few days before I do a proper announced-on-twitter blog post :) But http://g.wyz.nl/lql will be redirecting to my first shortened URL, a few days from now.
Again: because of the way shorturl_init() is set up, http://g.wyz.nl/lql and http://roderik.muit.nl/lql will both resolve and redirect to the same URL.
But my point is, that I want the /shorten page to present me with the http://g.wyz.nl/lql URL. And the url() function does not even know that this hostname exists. Your code can take it from the 'shorturl_domain' variable.

So... did that speech sound like a convincing argument? :-)

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

@irakli:

The patch right now belongs in shorten. That is because shorturl.module does not have a function to return a completed shortened URL.
However I think it should. so I created #628958: Request function to return full shortened URL.

(If that is implemented, this module can just call a function in shorturl.module to get the full URL. Which is conceptually cleaner. And moves the above discussion to from shorten.module to shorturl.module again... because Shorten can then just make one function call and be done with it :)

So I am curious what you think of my argument that my patch (i.e. using the 'shorturl_domain' variable) is actually the right approach :)

icecreamyou’s picture

icecreamyou commented

@roderik, url() is the way to go. The $options['base_url'] parameter of url() should be used to change roderik.muit to g.wyz. In theory, $options['prefix'] should get around the language problem, but from looking through the code in url(), it looks like language_url_rewrite() might override that. I'm not sure whether we care, as I have no idea when that function exists. A faux $language object is probably not necessary though.

Having said that, I rather prefer the solution suggested in #5 (#628958: Request function to return full shortened URL). If Irakli prefers that approach as well, then this issue can be marked as duplicate.

But to follow up: your assumption about url() is incorrect, in that url() does not guarantee a valid Drupal page. The obvious counter-example is that it can take external pages. But try url('404') for example -- it will return a link to where the page "404" would be on your site if it existed. The URL function doesn't look for existing pages. By default, it just figures out where Drupal is installed and tries to make sure local URLs point to the correct installation. However, it's flexible enough that any URL can be passed through or returned without contrib devs having to worry about PHP/system variables -- including, in this case, a URL from another domain.

I don't really care whether there's a legitimate use case for a URL shortener on localhost or not (or anywhere else for that matter). As a developer, I can't assume that someone won't want to use it there, and "it's not supported because your idea is dumb" is not a valid response. ;-)

icecreamyou’s picture

icecreamyou commented
Status: Needs work » Postponed

Considering that irakli has agreed to the concept in #628958: Request function to return full shortened URL, I'm going to change the purpose of this issue to switch to using the Short URL-provided full-URL function once that gets committed.

icecreamyou’s picture

icecreamyou commented
Category: bug » task

Er, yeah.

icecreamyou’s picture

icecreamyou commented
Status: Postponed » Closed (duplicate)

Marking duplicate in favor of #628958: Request function to return full shortened URL.