Posted by mfm on January 21, 2009 at 9:15am
| Project: | Smileys |
| Version: | 6.x-1.0 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
Smileys seems to use the absolute URL for the <img src="...">-part. It should use the relative URL (/sites/all/modules/smileys/packs/Tango/face-grin.png) instad of the full URL (https://www.menzer.net/sites/all/modules/smileys/packs/Tango/face-grin.png).
Comments
#1
No, it should use absolute URLs. Consider your content in RSS feeds, then it'll try to find Smileys image path relatively(in case we use relative path). If this is relative, it won't work! Happens often with screenshots/images on http://drupal.org/planet.
#2
Can't this be an option in the settings, instead of Gurpartap making the decision for all users of the module?
Consider the application:
A site has smileys enabled exclusively for the comment field of forums. There is no RSS feed for the comments in the forums, so all pageviews are at the site itself. There's a block on the user's profile page that shows the latest comments. Since the site uses SSL, but the cached content used a base path of http, the site throws annoying insecure errors.
Sure, this is an edge case, but as long as the alt text of the image is sufficiently-descriptive, I would be just fine with broken images in the RSS feeds.
#3
hmm, ok.
marked #403534: Smilie delivered over ssl as duplicate.
#4
Looking around at some issues*, it seems that the general consensus is that:
- modules like these should in fact always output relative URLs
- the RSS aggregator outputting broken URLs, is a bug that should be fixed in the RSS aggregator. (i.e. not in this module).
That is why I'm not too keen on making a configurable admin option. I just introduced a constant 'SMILEYS_ABSOLUTE_URLS' in the module which I will set to FALSE on my own site. My opinion is also that it should be FALSE by default, but I have kept it at TRUE in my patch so that it won't break 'compatibility' with earlier module releases.
* like #395764: Aggregator: Convert all relative URLs to absolute URLs in feed items, #88183: Relative urls in feeds
P.S. if you want to fix broken links in RSS feeds, you can apparently also install the pathologic module. (I haven't tried.)
#5
Wouldn't it not be better to figure out dynamically if the icon is shown on a webpage and if it's shown in RSS feeds? Every content that is cached in cache_filter need to be relative for not breaking with http/s...
#6
I agree that the problem with relative URLs in RSS feeds should be handled there.
Relative URLs are a Good Thing (tm) for most other cases so IMHO this should be
what modules create, especially if it is going to be cached in any way.
I know another real-world case where the absolute links created by the smileys module caused problems:
In a loadbalanced setup where the external port 80 is mapped to a different port on the
actual apache server we ended up with absolute links in the cache that contained the internal port.
It looks like the hosting company created these cache entries when doing tests from behind the load balancer.
This would not have been a problem if relative links had been used.
#7
I'm running into the problem that my emoticons on the deployed (production) site aren't found since the "absolute path" is used for all the smileys and that absolute path involves localhost and an odd port, among other differences. Other modules moved to production without issue. I spent a lot of time working out which to make active, which to include in the selection panel and which to "hide" (to keep it from being an excessively long list and to generally reserve some emoticons for admin purposes). So I'm going to try to figure out how to correct the path used by the module without having to "export" (will only work on my development server) and re-import / delete the old, etc. This could be a bit of a headache for a lot of users moving from one server to another. Hmmm...
Okay. Just worked out where the absolute path is stored... pretty wasteful, imho, to have it stored for each smiley in the smileys table, but it wasn't hard to export that table (using phpMyAdmin) to an SQL file, then edit it locally to change all "mysite.localhost" to "default" in the saved path. Now it should work since I imported that file and the table now reflects the correct location of the files. Phew... One glitch down, another who-knows-how-many to go. ;-)
Well... I thought that would solve the problem. But the smileys still aren't showing up. :-( The odd thing is that they are shown as "broken" links, even though I've got the path set correctly. If I try to "open in new window" one of the broken images, the path shown is correct (of course it's the search404 page since I have my site set up to not allow directly browsing such files, I guess; but that's the same on my dev site, where the images display fine... why aren't they showing up on my pages on the production server, then? The permissions for the files (if I look in my FTP client) seem okay. Grrr... Smileys are nice, but it's hard to justify spending too much time on getting them working when there are more important things to worry about.
http://www.frankfurtnavigator.com/sites/default/files/smileys/misc_smili...
#8
Hmnn... now I'm hesitant to install this module. I use Domain Access for multiple sites and several caching schemes for performance and an absolute url for every emoticon for every subdomain is likely to cause havoc.
Has anyone successfully fixed the absolute/relative url issue?
#9
Hmnn... now I'm hesitant to install this module. I use Domain Access for multiple sites and several caching schemes for performance and an absolute url for every emoticon for every subdomain is likely to cause havoc.
Has anyone successfully fixed the absolute/relative url issue?
#10
I fixed it for my use case, on july 12, 2009. See #4. The patch is there. You can apply it and change the constant to FALSE.
What this needs to move forward, is comments from other people on the approach to take. Because it's not my module.
The only comment in that respect was in #5, and the module author has not responded.
So:
- are we in agreement that this module should use relative URLs, in principle?
- are we in agreement that the default module should change its behaviour to the 'principally right behaviour', that is, using relative URLs?
- fixing the issue for RSS feeds (as in #88183: Relative urls in feeds) has been postponed until D8. So... as a 'temporary fix', do we make a config option for D6 and D7, clearly documented so people know what this is about?
I'm willing to roll a patch with an option in an admin screen, if noone else does it (whenever I get to it, that is)...
...but people will need to tell me that this approach is going to be adopted -- otherwise it's a waste of time.
@ #5 / hass: I do not know about RSS feeds & caching. Do you mean that no RSS feed content is ever cached in cache_filter?
Because otherwise your reply is a contradiction in terms, right?
#11
I for one, vote for relative URL's (RSS feeds need to be fixed, IMO, to grab the absolute URL at the time the feed is generated.) I will adopt the patch.
#12
Meh. I forgot to put quotes around the constant definition. That provokes php notices.
#13
Use theme_image for image tags, please.
#14
Converted to theme_image for image tags...
:)
#15
Now you're defining $base, but not using it. your theme() call always outputs relative URLs.
Please
- either reimplement the option to still output absolute URLs
- or just get rid of all the constant and all references to $base.
I'm fine with the second thing. If people want absolute URLs, they should fix aggregator, as per #88183: Relative urls in feeds. But note that this causes a regression in the Smileys module, and the maintainer may not commit the change.
(Though I don't know if that makes much of a difference - nobody seems to be committing anything anyway...)
#16
how to fix this problem in D6? the actual problem