Development
Threads by month
- ----- 2026 -----
- June
- May
- April
- March
- February
- January
- ----- 2025 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2024 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2023 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2022 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2021 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2020 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2019 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2018 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2017 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2016 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2015 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2014 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2013 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2012 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2011 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2010 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2009 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2008 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2007 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2006 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2005 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
September 2005
- 78 participants
- 615 discussions
[drupal-devel] [feature] Display comments on own tab like wikipedia (patch attached)
by nedjo 13 Sep '05
by nedjo 13 Sep '05
13 Sep '05
Issue status update for
http://drupal.org/node/24804
Post a follow up:
http://drupal.org/project/comments/add/24804
Project: Drupal
Version: cvs
Component: comment.module
Category: feature requests
Priority: normal
Assigned to: chx
Reported by: moshe weitzman
Updated by: nedjo
Status: patch (code needs review)
+1 in principle. (I've looked over but not applied the code.)
(Optional) tabbed display is a useful enhancement and justifies the
extra configuration option. I also like the new comment URL.
nedjo
Previous comments:
------------------------------------------------------------------------
Sat, 11 Jun 2005 20:57:10 +0000 : moshe weitzman
Attached please find a demonstration patch which moves comment display
to its own tab on the node view page. Admin may choose this new view or
old view.
The patch needs work still:
- comment control form redirects to the node page after submit. Should
be comment page
- same for comment posting form
- probably more
I hope someone gets inspired to finish this patch. I won't revisit this
for a while.
------------------------------------------------------------------------
Sat, 11 Jun 2005 21:00:56 +0000 : moshe weitzman
Attachment: http://drupal.org/files/issues/discuss.patch (3.87 KB)
with attachment this time
------------------------------------------------------------------------
Sat, 11 Jun 2005 23:24:12 +0000 : moshe weitzman
oh yeah - also on the TODO list is to comment URLs to always point to
comment.module and never to node/x#comment-cid or node/x#comment-new
and so on. then comment.module can work out whether to show you the
discuss tab or the traditional view.
------------------------------------------------------------------------
Sun, 12 Jun 2005 00:56:02 +0000 : Boris Mann
I like the concept. Would want this to be part of workflow settings, so
that comment-tab could be enabled/disabled per content type.
------------------------------------------------------------------------
Sun, 12 Jun 2005 02:20:42 +0000 : Jaza
I like the idea - big +1. This is a must for sites that have long
articles and/or many comments - viewing the node is enough bandwidth as
it is, without all the comments getting displayed as well. Also improves
usability, IMO.
Would also like to see pagination of the comments tab (I'm sure you've
thought of this already, Moshe). Something similar to A List Apart
[1]'s system, e.g. node/x/discuss/2 (page 2 of that node's comments).
Or, if the existing pager system demands it, node/x/discuss?page=2.
[1] http://www.alistapart.com/
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:41:42 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_0.patch (3.87 KB)
Let's discuss! Like do we want to have a reply form on the node display
even if the comments are on the discuss page?
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:50:13 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_1.patch (3.58 KB)
I have uploaded moshe's patch. Let's try again.
------------------------------------------------------------------------
Tue, 30 Aug 2005 11:22:18 +0000 : Bèr Kessels
I like this approach a lot. However, as i use drupal also for blogging,
would dislike my bog to behave the same. Thus there must be some -easy-
way to get around this. µ
The problem lies mainly in the fact that tabs are not administratable
in Drupals menu admin. And I think it is a very bad idea to add a
config option where to place the comments. We should go for one way,
teh default way, and offer a menu-admin of some sort to change this,
for bloggers and slashdot alikes.
How to achieve this? I see two options, but I really hope people see
more then one.:
- make local-tasks appear in the menu admin (gets a huge +1 from me)
It's a separate issue, though.
- add a config option to the comment admin to allow comments either as
tab or as thread under the post. IMO that config option should not set
a variable, that gets checked every hook_menu call, but it should alter
the menu-entry in the database for the tabs Its possible, I did that
before.
But, as said, I hope others have better ideas. for I really would love
this feature to get in. But I am afraid a huge lot of people will move
away from drupal if we allow tabs only.
------------------------------------------------------------------------
Tue, 30 Aug 2005 14:05:59 +0000 : chx
No, not tabs only. I know the drupal_gotos are giving you this
impression, but will fix in the evening. Also, there is only a
variable_get for node/nid pages and then again, variable_get is double
cached, it's very fast.
------------------------------------------------------------------------
Tue, 30 Aug 2005 16:19:22 +0000 : Bèr Kessels
I know variable is fast, but I was referring to:
&& variable_get('comment_task', FALSE) that is evaluated every node
view.
As well as the setting in the comment options. I am no fan of such
options; However, it seems in this case that setting is sortof
required, unfortunately. So unless others come up with better ideas,
just ignore my previous comment/post.
Another idea: put this in hook_nodeapi() $op == "settings". that way
this setting can be used per type. I can imagine that this makes no
sense for forums and blog entries, yet is very usefull for the books on
that same site.
------------------------------------------------------------------------
Tue, 30 Aug 2005 16:20:36 +0000 : Bèr Kessels
sorry, with "Another idea: put this in..." I meant to say "Another idea:
put the settings for tabs vs threads ..."
------------------------------------------------------------------------
Mon, 12 Sep 2005 22:24:22 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_2.patch (6.71 KB)
Comment URL is introduced. Instead of node/$nid#comment-cid we have
comment/$nid#comment-cid
Other minor fixes.
------------------------------------------------------------------------
Tue, 13 Sep 2005 00:31:31 +0000 : Boris Mann
Comment URLs are great, except I'd love to have a *full* comment URL --
e.g. comment/$nid/$cid. The probably with anchor tags is that they are
not visible at the server level -- i.e. not tracked in referrers, etc.
etc.
Don't want to hijack this issue, so feel free to ignore for now, but
wanted to bring up the issue from a search/web traffic/etc.
perspective.
1
0
Issue status update for
http://drupal.org/node/21924
Post a follow up:
http://drupal.org/project/comments/add/21924
Project: Drupal
-Version: 4.6.0
+Version: cvs
-Component: node.module
+Component: file system
Category: bug reports
Priority: normal
Assigned to: walkah
Reported by: gregmills
Updated by: walkah
Status: patch (ready to be committed)
this is actually a file.inc (file_create_url) issue. (and applies to
HEAD as well)
walkah
Previous comments:
------------------------------------------------------------------------
Tue, 03 May 2005 15:26:17 +0000 : gregmills
I was trying to work this out on the forums and Dries Buytaert suggested
I post a bug report so here's what's going on:
If I set the "Download Method" setting to "Public" then the link in the
RSS enclosure for my Podcast is absolute and fully qualified but lands
on a Drupal "page not found" error page. This setting also breaks all
the photos in my galleries. Furthermore, if I put the location of the
image on the server in the address bar of my browser I get the Drupal
"Page Not Found" error page. Isn't this the opposite of how "Public" is
supposed to work.
Setting the download method to "Private" fixes the galleries but then
all my RSS enclosures are converted to relative links. These don't seem
to work at all in Podcast recieving programs (I'm testing with iPodder
and Doppler). I checked the RSS of some of the other Podcasts that I
listen to and all their enclosures have absolute links to the media
files.
------------------------------------------------------------------------
Wed, 11 May 2005 02:27:41 +0000 : gregmills
Would it be possible for me to just patch the code where the following
line is generated?
If I could just get it to add in my full URL I think everything would
work fine.
ex:
------------------------------------------------------------------------
Wed, 11 May 2005 02:29:03 +0000 : gregmills
Sorry,
Change:
<enclosure url="system/files?file=050805.mp3" length="5205243"
type="audio/mpeg"/>
to:
<enclosure
url="http://www.miamichurch.org/system/files?file=050805.mp3"
length="5205243" type="audio/mpeg"/>
------------------------------------------------------------------------
Sat, 14 May 2005 02:49:34 +0000 : gregmills
I found it and changed it myself but it didn't alter the behavior.
With the download method set to "private" iPodder finds the feeds and
see the "episodes." But when it tries to download the episode it just
says "downloading" but never makes any progress. I can enter the URL
exactly as it appears in the enclosures into my web browser and it
downloads just fine.
I downloaded the XML from a feed that I knew worked and I substituted
the URL from Drupal's enclosure into it. Then I saved it back to my
server and subscribed to it in iPodder. The result was exactly the
same. It still says "Downloading" but never makes any progress. It
doesn't work in Doppler either.
With the download method set to "public" iPodder downloads Drupal's
"Page Not Found" page with the MP3's filename. This setting also breaks
all the photos in my gallery.
I've spent a lot of time trying to isolate the cause of this and I'm
getting nowhere. I've also been searching Google and fishing in
Drupal's forum for a working example of Podcast in Drupal and I can't
find one. Everyone doing a Podcast is currently using Feedburner to
create the feed. I upgraded the priority to critical because I believe
that this is important functionality that may be completely unusable. I
hope that's alright.
Any help is greatly appreciated and I've turned on mail so I can work
directly with whoever will look into this. My programming experience is
very limited but if I can help troubleshoot in any way then I am
available.
Thanks
------------------------------------------------------------------------
Wed, 01 Jun 2005 21:36:15 +0000 : zirafa
Check out echoditto's podcast at radio.echoditto.com. It might help to
ask them how they got theirs working. As for this podcasting problem,
it seems like there should be a way to fix it by inserting the site_url
variable or something right before the path so that the result is a
completely absolute path when the enclosure url is created. I don't
have the means to immediately fix this but I can see how this
completely breaks anybody that is trying to use drupal to make podcasts
(including myself). I'll work with you on pushing this and fixing this,
Greg, but my drupal coding knowledge is pretty basic. You can get in
touch with me by emailing me directly.
Farsheed
------------------------------------------------------------------------
Thu, 23 Jun 2005 08:55:32 +0000 : Bèr Kessels
Could you please share the url of the feed? without that it remains wild
guessing.
------------------------------------------------------------------------
Thu, 23 Jun 2005 08:56:30 +0000 : Bèr Kessels
oh, and IMHO this is not "critical". Maybe for you it is, but "critical"
is meant as "critical for drupal"
------------------------------------------------------------------------
Thu, 30 Jun 2005 23:14:28 +0000 : gregmills
Critical... Normal... It doesn't matter. Either way it's been a month
and a half since I first posted. I've managed to piece together
workarounds for several problems I discovered in the process of setting
up my feed. It's working now and I'm focusing on making the RSS output
compatible with iTunes.
Unfortunately, I'm not a PHP programmer so most of my workarounds
involve taking out dynamic references in the source and hard-coding it
for my site. The fixes aren't transferable so I don't really have
anything I can share back to the community. This also takes me out of
the main development trunk so I don't know what's going to happen when
I apply the next upgrade.
I've gotten a little help by posting in the forums and posting bug
reports but for the most part I've been on my own in fixing problems
I've found with Podcasting in Drupal. If the community wants this
software to spread the community is going to have to become more
diligent about addressing issues like this in a timely manner.
Podcasting support was a key factor in my decision to use Drupal and
this site was not shy about advertising it as a feature. Still, I don't
know of any Podcast created entirely in Drupal other than my own. All
the ones I've seen use a third party web service.
------------------------------------------------------------------------
Thu, 30 Jun 2005 23:27:04 +0000 : chx
You speak a lot and tell us little. Ber asked for the feed URL but you
have not given it.
We do not know anything about your setup. Webserver, PHP version,
Drupal base URL etc.
Before criticizing the community, create a proper bug report. And guess
what -- if you google on good bug report you'll get two excellent
documents in the top three...
------------------------------------------------------------------------
Fri, 01 Jul 2005 22:47:38 +0000 : gregmills
None of that matters now. I messed around in the code for a couple of
weeks and finally got it to work on my own.
Check the date on the original report, it was almost two months ago.
I'm not criticizing anyone. This is free software so I don't expect any
kind of "customer" support. I'm simply recommending that if you want the
install base for Drupal to grow the response time to requests for help
is probably something that needs to be considered. You can take that as
a criticism or you can take it as advice.
Rest assured, you won't be getting anymore improper bug reports from
me.
If anyone is curious there is an actual working Drupal generated
Podcast feed at:
http://www.miamichurch.org/taxonomy/term/11/0/feed
That's mine, and it's the only one I know of.
------------------------------------------------------------------------
Sun, 03 Jul 2005 07:52:46 +0000 : Bèr Kessels
Greg,
You must really try to be more polite when posting in the drupal
issues. Your comment about us no knowing what wwe should do to get our
software spread is insulting, and so is your comment about our timing.
If you dont like the way drupal works, then change it, or move ver to
another community. Insulting people definately does not work.
Oh, and BTW, afaik drupals uploads are used as aclosed files in any
feeds, so we automagically have podcasting!
------------------------------------------------------------------------
Tue, 05 Jul 2005 02:40:29 +0000 : gregmills
You guys are totally reading way too much into this. I do appreciate
your offer for assistance. It's just that it's a month and a half too
late. There's no hard feelings and I intend to continue to use Drupal.
I just think perhaps we should look at how issues like mine are
addressed.
I do not believe that Podcasting works "automagically" in Drupal. If it
did, I think you'd be able to find more people than just me doing it. In
fact, it would probably make Drupal one of the leading platforms for
Pocasting. Instead of getting all indignant with me about the tone that
you're reading into my comments, why doesn't somebody just look into
this?
------------------------------------------------------------------------
Tue, 09 Aug 2005 14:06:41 +0000 : dumell
If your Drupal 4.6 has "access control" set to "public", podcasting will
work out-of-the-box automagically. Great.
But if your site has "access control" set to "private" - and there may
be very good reasons to user this setting - you will probably not be
able to get podcasting to work at all.
And changing access control setting once you are up and running will
probably mess things up.
I have gone down the same road as Greg on this one. First I noted that
if you use "public", your RSS feed will have absolute addresses to the
MP3 files, like this:
<enclosure url="http://example.com/system/files?file=test.mp3"
length="223680" type="audio/mpeg" />
and if you have "access control" set to "private" the links will be
relative and your RSS will contain something like this:
<enclosure url="/system/files?file=test.mp3" length="223680"
type="audio/mpeg" />
In this case, using relative url's, podcasting clients like ipodder and
doppler will not be able to download your MP3 files. I presumed the
fault was the relative url, although the XML file started with a base
url definition like:
<rss version="2.0" xml:base="http://example.com">
However, tweaking the upload.module (line 306 in version 1.31.2.5
2005/05/25) to force a "http://example.com/" to be inserted before the
url to make it absolute, did not help. And yes, anonymous users have
access to files.
So I tried to download the file directly from the original HTML node
with IE. On the HTML page of the node, the base url is defined as
"http://example.com" and the link to the MP3 file is relative as
"system/files?file=test.mp3". If I place the pointer on the link, IE
tells me that the url to the file is:
"http://example.com/system/files?file=test.mp3", no surprises here.
When I click on the link to the MP3 file, a IE file download requestor
appares saying "file name: text.mp3". And if I confirm, the file is
downloaded correctly.
But if I right click on the link and use copy shortcut
"http://example.com/system/files?file=test.mp3", IE opens a file
requestor and tells me the name of the file is "files?file=test.mp3".
This is a suprise. And if I confirm this, IE will tell me that it can
not download the file because it is "not albe to open the internet
site".
So, with a base url defined and a relative link to the file in the html
page, I can download the file, but if I try to paste the absolute path
to the file into IE, I can not download the file.
With Firefox both alternatives work.
Unfortunatelly, it seems both iPodder and Doppler runs into the same
problem that IE does. Switching of "clean url's" does not make any
difference.
Is the HTTP request to drupals access control mechanism (that
system/files?file= thingie) getting corrupt in some way, or Drupal
returning a HTTP respons that is not accepted by most clients? We are
using Drupals multi-site feature and this might be adding to our
problems. The problem does not seem to be in upload.module but perhaps
in node.module?
------------------------------------------------------------------------
Tue, 09 Aug 2005 14:43:58 +0000 : dumell
Obviously I tried to write too much text at once :)
If your access control is set to public, the absolute links will not
look like in my example above, since they will not contain that
"/system/files?file=" -thingie. It would instead look something like
"http://example.com/files/test.mp3". And this is important since
requesting the file TROUGH Drupal instead of directly from the web
server is what seems to be causing the problem for many or all
(podcast) clients, atleast in a multi-site setup.
------------------------------------------------------------------------
Thu, 08 Sep 2005 14:46:17 +0000 : Boris Mann
I can confirm this.
This is the same issue with *all* enclosures if files are set to
"private". Something is busted somewhere, this needs to be fixed for
both 4.6 and 4.7.
------------------------------------------------------------------------
Tue, 13 Sep 2005 04:27:32 +0000 : walkah
Attachment: http://drupal.org/files/issues/file-create_url.patch (625 bytes)
attached patch fixes (by using full url - just like "public files"
does).
1
0
Issue status update for
http://drupal.org/node/4109
Post a follow up:
http://drupal.org/project/comments/add/4109
Project: Drupal
Version: cvs
Component: base system
Category: bug reports
Priority: normal
Assigned to: Eric Scouten
Reported by: Eric Scouten
Updated by: Geary
Status: patch (code needs review)
+1 on this latest patch. I had a nagging problem on a fresh install of
4.6.3 in a subdirectory on a TextDrive account. If I went into
admin/user, then selected 'edit' on the first user, made a change and
saved it, I got the PHPSESSID in the URL. This patch fixed it for both
PHP4 and PHP5.
Eric's earlier patch fixed it for PHP5 but did not fix it for PHP4.
(Sorry Eric!)
Geary
Previous comments:
------------------------------------------------------------------------
Thu, 13 Nov 2003 00:22:39 +0000 : Eric Scouten
Some URLs get ?PHPSESSID=(whatever) added to them. I find this
distracting and unnecessary for anonymous users. Submitting patch #147
for consideration to remove this issue.
------------------------------------------------------------------------
Fri, 05 Dec 2003 14:39:25 +0000 : Kjartan
This is a local PHP configuration issue. Don't see it as something
Drupal should mess with, the less PHP settings Drupal messes with the
better.
------------------------------------------------------------------------
Fri, 05 Dec 2003 15:11:33 +0000 : killes(a)www.drop.org
Could you elaborate which config setting one has to tweak?
------------------------------------------------------------------------
Fri, 05 Dec 2003 15:18:11 +0000 : Anonymous
php_flag session.use_trans_sid off
php_flag session.use_only_cookies on
The first will disable the automatic addition of the session ID to
internal URLs, the second will remove support for passing session IDs
in URLs. See http://php.net/session [1] for more info. BTW the above
settings are for Apache, in PHP.ini you should not add the php_flag
part, and you should use = inbetween the name and value...
[1] http://php.net/session
------------------------------------------------------------------------
Mon, 07 Jun 2004 22:42:06 +0000 : shane
It seems that Drupal (via the .htaccess file) already "messes with" a
bunch of PHP settings. For example, my Drupal install "came with"
these messed with settings:
php_value register_globals 0
php_value track_vars 1
php_value short_open_tag 1
php_value magic_quotes_gpc 0
php_value magic_quotes_runtime 0
php_value magic_quotes_sybase 0
php_value arg_separator.output "&"
php_value session.cache_expire 200000
php_value session.gc_maxlifetime 200000
php_value session.cookie_lifetime 2000000
php_value session.auto_start 0
php_value session.save_handler user
php_value session.cache_limiter none
php_value allow_call_time_pass_reference On
I'm not sure that adding two more "php_value" flags to the stock
.htaccess file is going to arguably mess with more than already is.
Personally I find it extremely un-professional and very un-clean to
have funky PHPSESSID url strings tacked onto a majority of my URLs.
I've worked extremely hard to implement clean URLs (above and beyond
the already great "clean url" feature of Drupal - I don't like them
being turned into muck.
I'd vote that these two flags be added to the .htaccess file that is
distributed with Drupal. However, I'm uncertain of the overall true
impact of making these changes, so barring an technical reason - I
would absolutely request this as a feature. I've implemented these in
many of my production sites already (just today) - we'll see what
impact they have.
------------------------------------------------------------------------
Tue, 08 Jun 2004 02:56:00 +0000 : marky
It's a performance thing. You need to remember that while not everyone
has edit access to php.ini, those of us that do control our own servers
see no need for .htaccess directives that set php options.
>From the Apache docs [2]:
"Apache will look in every directory for .htaccess files. Thus,
permitting .htaccess files causes a performance hit, whether or not you
actually even use them! Also, the .htaccess file is loaded every time a
document is requested."
Ok, I'm lucky in that I control my own server, so I can effectively
remove the .htaccess file and move its config options to php.ini and
the vhost section of httpd.conf, thus speeding up the responsiveness of
my server.
But if you don't have that option there's nothing wrong with adding
those directives to your directory .htaccess file. That's what it's for
- it allows directory specific config options for those that don't have
access to their server config.
As Kjartan said, "the less PHP settings Drupal messes with the better".
I've got to agree - as far as I'm concerned it's already big enough. The
"php_value short_open_tag 1", "php_value session.auto_start 0" and
"php_value session.cache_limiter none" for example serve little
purpose, as they mirror the installation default settings (apart from
the last, where the available options are: nocache, private or public).
If you don't have the choice, use .htaccess, and add whatever
directives you see fit. But please remember that not everyone uses it.
Anyway, just my 2 beads.
[2] http://httpd.apache.org/docs-2.0/howto/htaccess.html#when
------------------------------------------------------------------------
Tue, 08 Jun 2004 04:38:32 +0000 : cel4145
Why not add them, but commented out with an explanation? When I
encountered this problem, I had to spend a little time on drupal.org
finding the solution. However, I am familiar with the .htaccess file
from doing installs. If it had been included there commented out, I
would have probably remembered it. After all, this is much the same
way that httpd.conf functions with it's many, optional, commented out
configuration options. Just makes it easier when a non-standard
configuration option has to be used.
Besides, I've also seen many Drupal sites with this problem. I suspect
that some of those sites would have implemented these flags if they had
been an option in the .htaccess file. And it looks bad. People that
visit the sites that don't know any better will assume it's Drupal, not
the lack of proper PHP settings.
------------------------------------------------------------------------
Tue, 08 Jun 2004 10:39:27 +0000 : killes(a)www.drop.org
It wouldn't hurt if Drupal had a better documented and possibly extended
.htaccess file.
Those who whish not to use it should really not care...
session.save_handler = user should probably included as well.
------------------------------------------------------------------------
Tue, 08 Jun 2004 12:17:06 +0000 : robertDouglass
If you use these settings:
php_flag session.use_trans_sid off
php_flag session.use_only_cookies on
won't it make it so that people with cookies disabled cannot have
session state? And if the PHPSESSID is in the URL, isn't that an
indication that the browser being used refused to take a cookie? I'd
really like clarification on this, if anyone knows the answers.
------------------------------------------------------------------------
Tue, 27 Jul 2004 13:25:46 +0000 : JonBob
The original issue is definitely by design, as these URLs need the
session ID to preserve the session when running under the given PHP
settings. A better-documented .htaccess is a different issue entirely.
------------------------------------------------------------------------
Thu, 19 Aug 2004 18:04:13 +0000 : gtoddv
It hasn't been mentioned in this thread or anywhere else that I can
find, but this session ID issue breaks validation. I don't know is this
will cause accessibility issues too. I have tried the .htaccess fix and
that doesn't change anything.
------------------------------------------------------------------------
Sat, 06 Nov 2004 07:17:21 +0000 : wazdog
The non-validating problem is another problem with your server setup.
PHP can be setup to used encoded &'s or not. Your server isn't if the
url isn't validating. You'd have to talk to your server admin to fix
that.
------------------------------------------------------------------------
Thu, 30 Dec 2004 20:32:35 +0000 : m_freeman2004
This is what you have to include in the .htaccess file:
# Fix for ?PHPSESSID in clean URLs
php_value session.use_trans_sid 0
php_value session.use_only_cookies 1
# End of fix
------------------------------------------------------------------------
Wed, 01 Jun 2005 19:07:53 +0000 : dr05
The code in .htaccess file:
# Fix for ?PHPSESSID in clean URLs
php_value session.use_trans_sid 0
php_value session.use_only_cookies 1
# End of fix
don't work!
Error: "Internal Server Error!"
------------------------------------------------------------------------
Thu, 14 Jul 2005 15:25:22 +0000 : jasonwhat
Anyone else looking into this? Can one get rid of the id added onto the
url but still keep the sessions going?
------------------------------------------------------------------------
Mon, 25 Jul 2005 06:16:22 +0000 : Eric Scouten
dr05, what version of Apache and what OS are you using? I've been using
this same patch with no problems on Mac OS X, Linux, and FreeBSD-based
servers using both Apache 1.3.x and 2.0.x.
Attached patch will apply these changes to .htaccess. (IMHO, this is
useful for the majority of users; those who are smart enough to
understand the performance issues can comment out these lines.)
------------------------------------------------------------------------
Mon, 25 Jul 2005 06:57:18 +0000 : clydefrog
You forgot the patch, Eric.
------------------------------------------------------------------------
Mon, 25 Jul 2005 08:02:26 +0000 : Eric Scouten
Attachment: http://drupal.org/files/issues/htaccess-no-session-ids.patch (383 bytes)
D'oh!
------------------------------------------------------------------------
Mon, 25 Jul 2005 08:45:48 +0000 : kbahey
There are cases where no parameter will fix this issue. On shared
hosting systems where you do not have access to php.ini, and the
hosting company has setup things in a way that they cannot be
overriden.
I found this out the hard way, after lobbying for the settings to be
included in the init_set() statements in the settings.php prior to 4.6
being released. They are simply ignored on some hosts. On other hosts,
you need to have a php.ini of your own, if suphp or SuExec is being
used.
In some cases, the only way to make things work is to compile your own
PHP as a CGI (detailed writeup) [3]. This can have disadvantages too,
such some performance impact, as well as excessive CPU utilization
(hosting company may not like it).
[3]
http://baheyeldin.com/drupal/how-to-get-rid-of-phpsessid-in-drupal-and-othe…
------------------------------------------------------------------------
Mon, 25 Jul 2005 08:49:31 +0000 : kbahey
Here is some references: issue 21170 [4], and a forum discussion [5].
[4] http://drupal.org/node/21170
[5] http://drupal.org/node/17947
------------------------------------------------------------------------
Sat, 30 Jul 2005 16:24:47 +0000 : ufku
in my case (shared hosting, ini_set() disabled) a php.ini file with only
this line in it solved the problem.
url_rewriter.tags = ''
you should place the php.ini in your site's base directory
------------------------------------------------------------------------
Sun, 07 Aug 2005 12:18:31 +0000 : djnz
Attachment: http://drupal.org/files/issues/rewrite_tags.patch (468 bytes)
Drupal already attempts to change a number of values using ini_set() in
settings.php, including session.use_trans_sid which should cure this
problem. Unfortunately as already noted elsewhere [6],
session.use_trans_sid only worked in ini_set() up to PHP 4.2.3.
Rather than change the whole philosophy and change this setting in
.htaccess or php.ini (neither of which work for many hosted set-ups),
surely the answer is to correct settings.php as described
The attached patch does just that - it is only one line.
Note that session ids are still appended to the url when redirecting by
Drupal eg after a POST. This is also easily patched [7], but as it is
done deliberately (why?) I have left it alone.
[6] http://drupal.org/node/17947#comment-36339
[7] http://drupal.org/node/25852#comment-45111
------------------------------------------------------------------------
Sun, 07 Aug 2005 13:26:17 +0000 : kbahey
PHPSESSIDs in the URL are a) ugly, b) negatively affects search engine
indexing, and most importantly, c) is a security risk that can enable a
malicious user to hijack your session.
We should work on a solution to eliminate this once and for all, and
for all configurations.
The current approach (putting some PHP parameters in settings.php, or
instructing users to change them in .htaccess or php.ini) has severe
shorcomings: Whatever we do, there are bound to be some configurations
that no amount of PHP parameters will cure.
Some hosts deny their users the ability to change some or all of the
above parameters.
I discovered this the painful way with many hosts I have used, and
friends and clients I have helped.
I think we need to rethink this whole session thing, in a way that we
are not dependant on idiosyncracies of various hosts and PHP
configurations.
Perhaps attempt to set a DRUPAL cookie and not maintain a session
unless we are able to set it. This way we know that we will not have a
PHPSESSID in the URL. If we are not able to set the cookie, then we
allow only anonymous access that is sessionless..
I am not sure if that will work or not: what other options do we have
here?
------------------------------------------------------------------------
Sun, 07 Aug 2005 22:02:19 +0000 : djnz
I am convinced that ini_set('url_rewriter.tags','') is the answer. I
find it hard to believe that any host would go to the trouble to
disable this (not just a matter of setting compile time paramaters, you
would have to hack the PHP source; and why would they?) - certainly no
cPanel host I have used does.
The reason the code currently in settings.php doesn't work is not
because hosts disable it, it is because PHP does not support it. RTFM.
------------------------------------------------------------------------
Sun, 07 Aug 2005 22:23:59 +0000 : djnz
Attachment: http://drupal.org/files/issues/trans_sid.patch (1.4 KB)
After further thought, there really is no point in appending the SID to
a redirect url, so the attached patch solves this problem too.
If anyone has a problem with session IDs in URLs after applying this
patch, I'll offer them free hosting myself. Maybe. ;)
------------------------------------------------------------------------
Mon, 08 Aug 2005 03:14:41 +0000 : kbahey
You are right about the fact that the use of PHPSESSID in the URL on a
redirect is totally unnecessary. That line should be removed from the
base code altogether, there is no excuse for it.
As for url_rewriter.tags, I am not 100% sure, but I tried it with
someone who was facing difficulties, and it did not work as far as I
can recall.
In any case, your solution should go in base. It cannot hurt, unless
some host has disabled it.
------------------------------------------------------------------------
Mon, 08 Aug 2005 20:09:40 +0000 : moshe weitzman
yeah, i think that is cruft.
------------------------------------------------------------------------
Mon, 08 Aug 2005 21:15:29 +0000 : kbahey
Attachment: http://drupal.org/files/issues/common.inc-trans_sid.patch (759 bytes)
Actually, since the code in common.inc should not fiddle with PHPSESSID
in URL in the first place, why not junk that whole section of the code
altogether?
A patch for common.inc is attached that removes this whole section. As
Moshe said, this is cruft.
------------------------------------------------------------------------
Tue, 09 Aug 2005 16:40:56 +0000 : djnz
Attachment: http://drupal.org/files/issues/both_sid_patches.patch (1.39 KB)
Yes, you are both right - I was being too kind to the code just FALSEing
it out, it should just be deleted. I don't want to lose the
url_rewriter.tags patch though so the attached patch has both.
1
0
Issue status update for
http://drupal.org/node/21924
Post a follow up:
http://drupal.org/project/comments/add/21924
Project: Drupal
Version: 4.6.0
Component: node.module
Category: bug reports
Priority: normal
-Assigned to: Anonymous
+Assigned to: walkah
Reported by: gregmills
Updated by: walkah
-Status: active
+Status: patch (ready to be committed)
Attachment: http://drupal.org/files/issues/file-create_url.patch (625 bytes)
attached patch fixes (by using full url - just like "public files"
does).
walkah
Previous comments:
------------------------------------------------------------------------
Tue, 03 May 2005 15:26:17 +0000 : gregmills
I was trying to work this out on the forums and Dries Buytaert suggested
I post a bug report so here's what's going on:
If I set the "Download Method" setting to "Public" then the link in the
RSS enclosure for my Podcast is absolute and fully qualified but lands
on a Drupal "page not found" error page. This setting also breaks all
the photos in my galleries. Furthermore, if I put the location of the
image on the server in the address bar of my browser I get the Drupal
"Page Not Found" error page. Isn't this the opposite of how "Public" is
supposed to work.
Setting the download method to "Private" fixes the galleries but then
all my RSS enclosures are converted to relative links. These don't seem
to work at all in Podcast recieving programs (I'm testing with iPodder
and Doppler). I checked the RSS of some of the other Podcasts that I
listen to and all their enclosures have absolute links to the media
files.
------------------------------------------------------------------------
Wed, 11 May 2005 02:27:41 +0000 : gregmills
Would it be possible for me to just patch the code where the following
line is generated?
If I could just get it to add in my full URL I think everything would
work fine.
ex:
------------------------------------------------------------------------
Wed, 11 May 2005 02:29:03 +0000 : gregmills
Sorry,
Change:
<enclosure url="system/files?file=050805.mp3" length="5205243"
type="audio/mpeg"/>
to:
<enclosure
url="http://www.miamichurch.org/system/files?file=050805.mp3"
length="5205243" type="audio/mpeg"/>
------------------------------------------------------------------------
Sat, 14 May 2005 02:49:34 +0000 : gregmills
I found it and changed it myself but it didn't alter the behavior.
With the download method set to "private" iPodder finds the feeds and
see the "episodes." But when it tries to download the episode it just
says "downloading" but never makes any progress. I can enter the URL
exactly as it appears in the enclosures into my web browser and it
downloads just fine.
I downloaded the XML from a feed that I knew worked and I substituted
the URL from Drupal's enclosure into it. Then I saved it back to my
server and subscribed to it in iPodder. The result was exactly the
same. It still says "Downloading" but never makes any progress. It
doesn't work in Doppler either.
With the download method set to "public" iPodder downloads Drupal's
"Page Not Found" page with the MP3's filename. This setting also breaks
all the photos in my gallery.
I've spent a lot of time trying to isolate the cause of this and I'm
getting nowhere. I've also been searching Google and fishing in
Drupal's forum for a working example of Podcast in Drupal and I can't
find one. Everyone doing a Podcast is currently using Feedburner to
create the feed. I upgraded the priority to critical because I believe
that this is important functionality that may be completely unusable. I
hope that's alright.
Any help is greatly appreciated and I've turned on mail so I can work
directly with whoever will look into this. My programming experience is
very limited but if I can help troubleshoot in any way then I am
available.
Thanks
------------------------------------------------------------------------
Wed, 01 Jun 2005 21:36:15 +0000 : zirafa
Check out echoditto's podcast at radio.echoditto.com. It might help to
ask them how they got theirs working. As for this podcasting problem,
it seems like there should be a way to fix it by inserting the site_url
variable or something right before the path so that the result is a
completely absolute path when the enclosure url is created. I don't
have the means to immediately fix this but I can see how this
completely breaks anybody that is trying to use drupal to make podcasts
(including myself). I'll work with you on pushing this and fixing this,
Greg, but my drupal coding knowledge is pretty basic. You can get in
touch with me by emailing me directly.
Farsheed
------------------------------------------------------------------------
Thu, 23 Jun 2005 08:55:32 +0000 : Bèr Kessels
Could you please share the url of the feed? without that it remains wild
guessing.
------------------------------------------------------------------------
Thu, 23 Jun 2005 08:56:30 +0000 : Bèr Kessels
oh, and IMHO this is not "critical". Maybe for you it is, but "critical"
is meant as "critical for drupal"
------------------------------------------------------------------------
Thu, 30 Jun 2005 23:14:28 +0000 : gregmills
Critical... Normal... It doesn't matter. Either way it's been a month
and a half since I first posted. I've managed to piece together
workarounds for several problems I discovered in the process of setting
up my feed. It's working now and I'm focusing on making the RSS output
compatible with iTunes.
Unfortunately, I'm not a PHP programmer so most of my workarounds
involve taking out dynamic references in the source and hard-coding it
for my site. The fixes aren't transferable so I don't really have
anything I can share back to the community. This also takes me out of
the main development trunk so I don't know what's going to happen when
I apply the next upgrade.
I've gotten a little help by posting in the forums and posting bug
reports but for the most part I've been on my own in fixing problems
I've found with Podcasting in Drupal. If the community wants this
software to spread the community is going to have to become more
diligent about addressing issues like this in a timely manner.
Podcasting support was a key factor in my decision to use Drupal and
this site was not shy about advertising it as a feature. Still, I don't
know of any Podcast created entirely in Drupal other than my own. All
the ones I've seen use a third party web service.
------------------------------------------------------------------------
Thu, 30 Jun 2005 23:27:04 +0000 : chx
You speak a lot and tell us little. Ber asked for the feed URL but you
have not given it.
We do not know anything about your setup. Webserver, PHP version,
Drupal base URL etc.
Before criticizing the community, create a proper bug report. And guess
what -- if you google on good bug report you'll get two excellent
documents in the top three...
------------------------------------------------------------------------
Fri, 01 Jul 2005 22:47:38 +0000 : gregmills
None of that matters now. I messed around in the code for a couple of
weeks and finally got it to work on my own.
Check the date on the original report, it was almost two months ago.
I'm not criticizing anyone. This is free software so I don't expect any
kind of "customer" support. I'm simply recommending that if you want the
install base for Drupal to grow the response time to requests for help
is probably something that needs to be considered. You can take that as
a criticism or you can take it as advice.
Rest assured, you won't be getting anymore improper bug reports from
me.
If anyone is curious there is an actual working Drupal generated
Podcast feed at:
http://www.miamichurch.org/taxonomy/term/11/0/feed
That's mine, and it's the only one I know of.
------------------------------------------------------------------------
Sun, 03 Jul 2005 07:52:46 +0000 : Bèr Kessels
Greg,
You must really try to be more polite when posting in the drupal
issues. Your comment about us no knowing what wwe should do to get our
software spread is insulting, and so is your comment about our timing.
If you dont like the way drupal works, then change it, or move ver to
another community. Insulting people definately does not work.
Oh, and BTW, afaik drupals uploads are used as aclosed files in any
feeds, so we automagically have podcasting!
------------------------------------------------------------------------
Tue, 05 Jul 2005 02:40:29 +0000 : gregmills
You guys are totally reading way too much into this. I do appreciate
your offer for assistance. It's just that it's a month and a half too
late. There's no hard feelings and I intend to continue to use Drupal.
I just think perhaps we should look at how issues like mine are
addressed.
I do not believe that Podcasting works "automagically" in Drupal. If it
did, I think you'd be able to find more people than just me doing it. In
fact, it would probably make Drupal one of the leading platforms for
Pocasting. Instead of getting all indignant with me about the tone that
you're reading into my comments, why doesn't somebody just look into
this?
------------------------------------------------------------------------
Tue, 09 Aug 2005 14:06:41 +0000 : dumell
If your Drupal 4.6 has "access control" set to "public", podcasting will
work out-of-the-box automagically. Great.
But if your site has "access control" set to "private" - and there may
be very good reasons to user this setting - you will probably not be
able to get podcasting to work at all.
And changing access control setting once you are up and running will
probably mess things up.
I have gone down the same road as Greg on this one. First I noted that
if you use "public", your RSS feed will have absolute addresses to the
MP3 files, like this:
<enclosure url="http://example.com/system/files?file=test.mp3"
length="223680" type="audio/mpeg" />
and if you have "access control" set to "private" the links will be
relative and your RSS will contain something like this:
<enclosure url="/system/files?file=test.mp3" length="223680"
type="audio/mpeg" />
In this case, using relative url's, podcasting clients like ipodder and
doppler will not be able to download your MP3 files. I presumed the
fault was the relative url, although the XML file started with a base
url definition like:
<rss version="2.0" xml:base="http://example.com">
However, tweaking the upload.module (line 306 in version 1.31.2.5
2005/05/25) to force a "http://example.com/" to be inserted before the
url to make it absolute, did not help. And yes, anonymous users have
access to files.
So I tried to download the file directly from the original HTML node
with IE. On the HTML page of the node, the base url is defined as
"http://example.com" and the link to the MP3 file is relative as
"system/files?file=test.mp3". If I place the pointer on the link, IE
tells me that the url to the file is:
"http://example.com/system/files?file=test.mp3", no surprises here.
When I click on the link to the MP3 file, a IE file download requestor
appares saying "file name: text.mp3". And if I confirm, the file is
downloaded correctly.
But if I right click on the link and use copy shortcut
"http://example.com/system/files?file=test.mp3", IE opens a file
requestor and tells me the name of the file is "files?file=test.mp3".
This is a suprise. And if I confirm this, IE will tell me that it can
not download the file because it is "not albe to open the internet
site".
So, with a base url defined and a relative link to the file in the html
page, I can download the file, but if I try to paste the absolute path
to the file into IE, I can not download the file.
With Firefox both alternatives work.
Unfortunatelly, it seems both iPodder and Doppler runs into the same
problem that IE does. Switching of "clean url's" does not make any
difference.
Is the HTTP request to drupals access control mechanism (that
system/files?file= thingie) getting corrupt in some way, or Drupal
returning a HTTP respons that is not accepted by most clients? We are
using Drupals multi-site feature and this might be adding to our
problems. The problem does not seem to be in upload.module but perhaps
in node.module?
------------------------------------------------------------------------
Tue, 09 Aug 2005 14:43:58 +0000 : dumell
Obviously I tried to write too much text at once :)
If your access control is set to public, the absolute links will not
look like in my example above, since they will not contain that
"/system/files?file=" -thingie. It would instead look something like
"http://example.com/files/test.mp3". And this is important since
requesting the file TROUGH Drupal instead of directly from the web
server is what seems to be causing the problem for many or all
(podcast) clients, atleast in a multi-site setup.
------------------------------------------------------------------------
Thu, 08 Sep 2005 14:46:17 +0000 : Boris Mann
I can confirm this.
This is the same issue with *all* enclosures if files are set to
"private". Something is busted somewhere, this needs to be fixed for
both 4.6 and 4.7.
1
0
[drupal-devel] [feature] Display comments on own tab like wikipedia (patch attached)
by Boris Mann 13 Sep '05
by Boris Mann 13 Sep '05
13 Sep '05
Issue status update for
http://drupal.org/node/24804
Post a follow up:
http://drupal.org/project/comments/add/24804
Project: Drupal
Version: cvs
Component: comment.module
Category: feature requests
Priority: normal
Assigned to: chx
Reported by: moshe weitzman
Updated by: Boris Mann
Status: patch (code needs review)
Comment URLs are great, except I'd love to have a *full* comment URL --
e.g. comment/$nid/$cid. The probably with anchor tags is that they are
not visible at the server level -- i.e. not tracked in referrers, etc.
etc.
Don't want to hijack this issue, so feel free to ignore for now, but
wanted to bring up the issue from a search/web traffic/etc.
perspective.
Boris Mann
Previous comments:
------------------------------------------------------------------------
Sat, 11 Jun 2005 20:57:10 +0000 : moshe weitzman
Attached please find a demonstration patch which moves comment display
to its own tab on the node view page. Admin may choose this new view or
old view.
The patch needs work still:
- comment control form redirects to the node page after submit. Should
be comment page
- same for comment posting form
- probably more
I hope someone gets inspired to finish this patch. I won't revisit this
for a while.
------------------------------------------------------------------------
Sat, 11 Jun 2005 21:00:56 +0000 : moshe weitzman
Attachment: http://drupal.org/files/issues/discuss.patch (3.87 KB)
with attachment this time
------------------------------------------------------------------------
Sat, 11 Jun 2005 23:24:12 +0000 : moshe weitzman
oh yeah - also on the TODO list is to comment URLs to always point to
comment.module and never to node/x#comment-cid or node/x#comment-new
and so on. then comment.module can work out whether to show you the
discuss tab or the traditional view.
------------------------------------------------------------------------
Sun, 12 Jun 2005 00:56:02 +0000 : Boris Mann
I like the concept. Would want this to be part of workflow settings, so
that comment-tab could be enabled/disabled per content type.
------------------------------------------------------------------------
Sun, 12 Jun 2005 02:20:42 +0000 : Jaza
I like the idea - big +1. This is a must for sites that have long
articles and/or many comments - viewing the node is enough bandwidth as
it is, without all the comments getting displayed as well. Also improves
usability, IMO.
Would also like to see pagination of the comments tab (I'm sure you've
thought of this already, Moshe). Something similar to A List Apart
[1]'s system, e.g. node/x/discuss/2 (page 2 of that node's comments).
Or, if the existing pager system demands it, node/x/discuss?page=2.
[1] http://www.alistapart.com/
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:41:42 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_0.patch (3.87 KB)
Let's discuss! Like do we want to have a reply form on the node display
even if the comments are on the discuss page?
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:50:13 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_1.patch (3.58 KB)
I have uploaded moshe's patch. Let's try again.
------------------------------------------------------------------------
Tue, 30 Aug 2005 11:22:18 +0000 : Bèr Kessels
I like this approach a lot. However, as i use drupal also for blogging,
would dislike my bog to behave the same. Thus there must be some -easy-
way to get around this. µ
The problem lies mainly in the fact that tabs are not administratable
in Drupals menu admin. And I think it is a very bad idea to add a
config option where to place the comments. We should go for one way,
teh default way, and offer a menu-admin of some sort to change this,
for bloggers and slashdot alikes.
How to achieve this? I see two options, but I really hope people see
more then one.:
- make local-tasks appear in the menu admin (gets a huge +1 from me)
It's a separate issue, though.
- add a config option to the comment admin to allow comments either as
tab or as thread under the post. IMO that config option should not set
a variable, that gets checked every hook_menu call, but it should alter
the menu-entry in the database for the tabs Its possible, I did that
before.
But, as said, I hope others have better ideas. for I really would love
this feature to get in. But I am afraid a huge lot of people will move
away from drupal if we allow tabs only.
------------------------------------------------------------------------
Tue, 30 Aug 2005 14:05:59 +0000 : chx
No, not tabs only. I know the drupal_gotos are giving you this
impression, but will fix in the evening. Also, there is only a
variable_get for node/nid pages and then again, variable_get is double
cached, it's very fast.
------------------------------------------------------------------------
Tue, 30 Aug 2005 16:19:22 +0000 : Bèr Kessels
I know variable is fast, but I was referring to:
&& variable_get('comment_task', FALSE) that is evaluated every node
view.
As well as the setting in the comment options. I am no fan of such
options; However, it seems in this case that setting is sortof
required, unfortunately. So unless others come up with better ideas,
just ignore my previous comment/post.
Another idea: put this in hook_nodeapi() $op == "settings". that way
this setting can be used per type. I can imagine that this makes no
sense for forums and blog entries, yet is very usefull for the books on
that same site.
------------------------------------------------------------------------
Tue, 30 Aug 2005 16:20:36 +0000 : Bèr Kessels
sorry, with "Another idea: put this in..." I meant to say "Another idea:
put the settings for tabs vs threads ..."
------------------------------------------------------------------------
Mon, 12 Sep 2005 22:24:22 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_2.patch (6.71 KB)
Comment URL is introduced. Instead of node/$nid#comment-cid we have
comment/$nid#comment-cid
Other minor fixes.
1
0
[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by nedjo 13 Sep '05
by nedjo 13 Sep '05
13 Sep '05
Issue status update for
http://drupal.org/node/30220
Post a follow up:
http://drupal.org/project/comments/add/30220
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
Priority: normal
Assigned to: Anonymous
Reported by: m3avrck
Updated by: nedjo
Status: patch (code needs review)
+1 on idea, js looks generally good, a few suggestions:
1. Instead of writing onsubmit via PHP, use a js addLoadEvent call.
2. Shouldn't the return false in isElementChanged be at the end
(outside the switch block)?
3. It's be nice to find a way to make messages like 'You have unsaved
changes.' translatable. Pass in global js variables via a t('') call?
4. onbeforeunload event should probably be in a if isJsEnabled test,
and should parallel drupal.js's event adding (see addLoadEvent).
5. Comments should be in standard format and in present tense, e.g.,
/**
* Checks to see if a form has been changed after the page loads
*/
6. e_ should be just e to match other js files, e.g., autocomplete.
nedjo
Previous comments:
------------------------------------------------------------------------
Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck
How many times have you been posting a comment or working on a new post
when you click on a link in another application and you navigate away
from the content you are editing, losing all of your changes?
Well I say we introduce a script, much similiar to that of Blogger,
that uses the Javascript window.onbeforeunload handler to prevent this
from happening. We can set this handler to call a function that
compares the contents of all forms when the page loads and then quickly
compares that to the current contents just before the user is about to
leave the page. If they have changed, we should prompt the user so they
don't lose their work. This would save many headaches and degrade nicely
for users without Javascript.
Additionally, this script should work with the newly introduced
JS-based upload feature to prevent navigating away as a file is being
uploaded as well.
If I have sometime I'll start work on a patch tomorrow, just wanted to
get the idea into the queue right now :)
------------------------------------------------------------------------
Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH
Sounds like a great feature to me. We regularly get authors and editors
telling us that they've lost the last hour of work by accidentally
navigating away, and I've even just had one of them email me about it a
few minutes ago, so it'd be great if you could put something together
for this.
Cheers
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC
Sounds great to me, too. I often have several tabs open doing stuff and
it's easy to lose my place and leave a page with unfinished edits.
Would this alert you when you're going to close an unsaved tab/window
as well?
Michelle
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles
Great for us multitaskers that rush and click the wrong thing
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:36:52 +0000 : chx
Very strong -1 as this is IE only.
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium
Konqueror already does this, +1 for the idea in general.
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:07:21 +0000 : webchick
Yeah, I think chx is correc that onbeforeunload is an extension
developed by MS [1] (it's not in the ECMAScript specification [2].
However, I found out that this is implemented in Mozilla since 1.7 [3]
(which corresponds to Firefox 1.0, I think?) and I've tested
Blogger.com's version of this functionality in:
Firefox 1.0.6 (working)
Firefox Deer Park Alpha 1 (working)
Opera 8.0.2 (working)
Safari 1.3 (does NOT work)
Safari 2.0.1 (working)
So it seems to be a 'de facto' standard if nothing else.
[1]
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun…
[2]
http://www.ecma-international.org/publications/standards/Ecma-262.htm
[3] http://www.mozilla.org/status/2004-03-01.html
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:09:15 +0000 : webchick
Also, I should clarify... that "does NOT work" in Safari doesn't display
any errors or anything, it just doesn't save the form results as
expected (so pre-Tiger Safari users are going to be used to this). So I
don't see any harm in including this functionality since it seems to
degrade gracefully in non-supporting browsers.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes)
And a patch is ready! Code was based on many example, including this [4]
and this [5] along with adding my own thoughts and what not ;)
[4]
http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O…
[5] http://www.blogger.com/app/scripts/formcheck.js
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB)
And here is the JS to throw in misc/.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB)
Better JS file attached without tabs. Also, tested and working great in
FF and IE6. Doesn't work in Opera 8, however no errors are present, I
believe this is just because the event handler is not supported. If
there is a work around for Opera, please let me know!
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB)
One more try with those tabs.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman
hmmm. i usually find these prompts annoying. any chance we can attach
the behavior only to the node and comment forms? those are the "high
risk" areas. what do others think?
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:42:39 +0000 : m3avrck
Yes, the JS is only loaded on the node edit/create pages, won't affect
any admin/login/etc form. Also, the prompt is unobstrusive and if you
goto save a node you don't get a warning that the contents have changed
because you are explicity saving the node (hence no reason!).
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:53:01 +0000 : Dries
I want some JS-folks to review the code.
------------------------------------------------------------------------
Mon, 12 Sep 2005 22:09:10 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_15.patch (1.94 KB)
After talking with Drumm on IRC, this patch makes the integration with
Drupal much simpler and it's off by default. Only turned on for the
node edit/change page right now but any module/form can easily add this
by passing a 'TRUE' parameter.
1
0
[drupal-devel] [feature] Display comments on own tab like wikipedia (patch attached)
by chx 13 Sep '05
by chx 13 Sep '05
13 Sep '05
Issue status update for
http://drupal.org/node/24804
Post a follow up:
http://drupal.org/project/comments/add/24804
Project: Drupal
Version: cvs
Component: comment.module
Category: feature requests
Priority: normal
Assigned to: chx
Reported by: moshe weitzman
Updated by: chx
Status: patch (code needs review)
Attachment: http://drupal.org/files/issues/discuss_2.patch (6.71 KB)
Comment URL is introduced. Instead of node/$nid#comment-cid we have
comment/$nid#comment-cid
Other minor fixes.
chx
Previous comments:
------------------------------------------------------------------------
Sat, 11 Jun 2005 20:57:10 +0000 : moshe weitzman
Attached please find a demonstration patch which moves comment display
to its own tab on the node view page. Admin may choose this new view or
old view.
The patch needs work still:
- comment control form redirects to the node page after submit. Should
be comment page
- same for comment posting form
- probably more
I hope someone gets inspired to finish this patch. I won't revisit this
for a while.
------------------------------------------------------------------------
Sat, 11 Jun 2005 21:00:56 +0000 : moshe weitzman
Attachment: http://drupal.org/files/issues/discuss.patch (3.87 KB)
with attachment this time
------------------------------------------------------------------------
Sat, 11 Jun 2005 23:24:12 +0000 : moshe weitzman
oh yeah - also on the TODO list is to comment URLs to always point to
comment.module and never to node/x#comment-cid or node/x#comment-new
and so on. then comment.module can work out whether to show you the
discuss tab or the traditional view.
------------------------------------------------------------------------
Sun, 12 Jun 2005 00:56:02 +0000 : Boris Mann
I like the concept. Would want this to be part of workflow settings, so
that comment-tab could be enabled/disabled per content type.
------------------------------------------------------------------------
Sun, 12 Jun 2005 02:20:42 +0000 : Jaza
I like the idea - big +1. This is a must for sites that have long
articles and/or many comments - viewing the node is enough bandwidth as
it is, without all the comments getting displayed as well. Also improves
usability, IMO.
Would also like to see pagination of the comments tab (I'm sure you've
thought of this already, Moshe). Something similar to A List Apart
[1]'s system, e.g. node/x/discuss/2 (page 2 of that node's comments).
Or, if the existing pager system demands it, node/x/discuss?page=2.
[1] http://www.alistapart.com/
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:41:42 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_0.patch (3.87 KB)
Let's discuss! Like do we want to have a reply form on the node display
even if the comments are on the discuss page?
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:50:13 +0000 : chx
Attachment: http://drupal.org/files/issues/discuss_1.patch (3.58 KB)
I have uploaded moshe's patch. Let's try again.
------------------------------------------------------------------------
Tue, 30 Aug 2005 11:22:18 +0000 : Bèr Kessels
I like this approach a lot. However, as i use drupal also for blogging,
would dislike my bog to behave the same. Thus there must be some -easy-
way to get around this. µ
The problem lies mainly in the fact that tabs are not administratable
in Drupals menu admin. And I think it is a very bad idea to add a
config option where to place the comments. We should go for one way,
teh default way, and offer a menu-admin of some sort to change this,
for bloggers and slashdot alikes.
How to achieve this? I see two options, but I really hope people see
more then one.:
- make local-tasks appear in the menu admin (gets a huge +1 from me)
It's a separate issue, though.
- add a config option to the comment admin to allow comments either as
tab or as thread under the post. IMO that config option should not set
a variable, that gets checked every hook_menu call, but it should alter
the menu-entry in the database for the tabs Its possible, I did that
before.
But, as said, I hope others have better ideas. for I really would love
this feature to get in. But I am afraid a huge lot of people will move
away from drupal if we allow tabs only.
------------------------------------------------------------------------
Tue, 30 Aug 2005 14:05:59 +0000 : chx
No, not tabs only. I know the drupal_gotos are giving you this
impression, but will fix in the evening. Also, there is only a
variable_get for node/nid pages and then again, variable_get is double
cached, it's very fast.
------------------------------------------------------------------------
Tue, 30 Aug 2005 16:19:22 +0000 : Bèr Kessels
I know variable is fast, but I was referring to:
&& variable_get('comment_task', FALSE) that is evaluated every node
view.
As well as the setting in the comment options. I am no fan of such
options; However, it seems in this case that setting is sortof
required, unfortunately. So unless others come up with better ideas,
just ignore my previous comment/post.
Another idea: put this in hook_nodeapi() $op == "settings". that way
this setting can be used per type. I can imagine that this makes no
sense for forums and blog entries, yet is very usefull for the books on
that same site.
------------------------------------------------------------------------
Tue, 30 Aug 2005 16:20:36 +0000 : Bèr Kessels
sorry, with "Another idea: put this in..." I meant to say "Another idea:
put the settings for tabs vs threads ..."
1
0
[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by m3avrck 13 Sep '05
by m3avrck 13 Sep '05
13 Sep '05
Issue status update for
http://drupal.org/node/30220
Post a follow up:
http://drupal.org/project/comments/add/30220
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
Priority: normal
Assigned to: Anonymous
Reported by: m3avrck
Updated by: m3avrck
Status: patch (code needs review)
Attachment: http://drupal.org/files/issues/drupal_15.patch (1.94 KB)
After talking with Drumm on IRC, this patch makes the integration with
Drupal much simpler and it's off by default. Only turned on for the
node edit/change page right now but any module/form can easily add this
by passing a 'TRUE' parameter.
m3avrck
Previous comments:
------------------------------------------------------------------------
Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck
How many times have you been posting a comment or working on a new post
when you click on a link in another application and you navigate away
from the content you are editing, losing all of your changes?
Well I say we introduce a script, much similiar to that of Blogger,
that uses the Javascript window.onbeforeunload handler to prevent this
from happening. We can set this handler to call a function that
compares the contents of all forms when the page loads and then quickly
compares that to the current contents just before the user is about to
leave the page. If they have changed, we should prompt the user so they
don't lose their work. This would save many headaches and degrade nicely
for users without Javascript.
Additionally, this script should work with the newly introduced
JS-based upload feature to prevent navigating away as a file is being
uploaded as well.
If I have sometime I'll start work on a patch tomorrow, just wanted to
get the idea into the queue right now :)
------------------------------------------------------------------------
Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH
Sounds like a great feature to me. We regularly get authors and editors
telling us that they've lost the last hour of work by accidentally
navigating away, and I've even just had one of them email me about it a
few minutes ago, so it'd be great if you could put something together
for this.
Cheers
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC
Sounds great to me, too. I often have several tabs open doing stuff and
it's easy to lose my place and leave a page with unfinished edits.
Would this alert you when you're going to close an unsaved tab/window
as well?
Michelle
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles
Great for us multitaskers that rush and click the wrong thing
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:36:52 +0000 : chx
Very strong -1 as this is IE only.
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium
Konqueror already does this, +1 for the idea in general.
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:07:21 +0000 : webchick
Yeah, I think chx is correc that onbeforeunload is an extension
developed by MS [1] (it's not in the ECMAScript specification [2].
However, I found out that this is implemented in Mozilla since 1.7 [3]
(which corresponds to Firefox 1.0, I think?) and I've tested
Blogger.com's version of this functionality in:
Firefox 1.0.6 (working)
Firefox Deer Park Alpha 1 (working)
Opera 8.0.2 (working)
Safari 1.3 (does NOT work)
Safari 2.0.1 (working)
So it seems to be a 'de facto' standard if nothing else.
[1]
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun…
[2]
http://www.ecma-international.org/publications/standards/Ecma-262.htm
[3] http://www.mozilla.org/status/2004-03-01.html
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:09:15 +0000 : webchick
Also, I should clarify... that "does NOT work" in Safari doesn't display
any errors or anything, it just doesn't save the form results as
expected (so pre-Tiger Safari users are going to be used to this). So I
don't see any harm in including this functionality since it seems to
degrade gracefully in non-supporting browsers.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes)
And a patch is ready! Code was based on many example, including this [4]
and this [5] along with adding my own thoughts and what not ;)
[4]
http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O…
[5] http://www.blogger.com/app/scripts/formcheck.js
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB)
And here is the JS to throw in misc/.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB)
Better JS file attached without tabs. Also, tested and working great in
FF and IE6. Doesn't work in Opera 8, however no errors are present, I
believe this is just because the event handler is not supported. If
there is a work around for Opera, please let me know!
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB)
One more try with those tabs.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman
hmmm. i usually find these prompts annoying. any chance we can attach
the behavior only to the node and comment forms? those are the "high
risk" areas. what do others think?
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:42:39 +0000 : m3avrck
Yes, the JS is only loaded on the node edit/create pages, won't affect
any admin/login/etc form. Also, the prompt is unobstrusive and if you
goto save a node you don't get a warning that the contents have changed
because you are explicity saving the node (hence no reason!).
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:53:01 +0000 : Dries
I want some JS-folks to review the code.
1
0
[drupal-devel] [feature] Prevent accidentally navigating away from pages where content has changed
by Dries 12 Sep '05
by Dries 12 Sep '05
12 Sep '05
Issue status update for
http://drupal.org/node/30220
Post a follow up:
http://drupal.org/project/comments/add/30220
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
Priority: normal
Assigned to: Anonymous
Reported by: m3avrck
Updated by: Dries
Status: patch (code needs review)
I want some JS-folks to review the code.
Dries
Previous comments:
------------------------------------------------------------------------
Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck
How many times have you been posting a comment or working on a new post
when you click on a link in another application and you navigate away
from the content you are editing, losing all of your changes?
Well I say we introduce a script, much similiar to that of Blogger,
that uses the Javascript window.onbeforeunload handler to prevent this
from happening. We can set this handler to call a function that
compares the contents of all forms when the page loads and then quickly
compares that to the current contents just before the user is about to
leave the page. If they have changed, we should prompt the user so they
don't lose their work. This would save many headaches and degrade nicely
for users without Javascript.
Additionally, this script should work with the newly introduced
JS-based upload feature to prevent navigating away as a file is being
uploaded as well.
If I have sometime I'll start work on a patch tomorrow, just wanted to
get the idea into the queue right now :)
------------------------------------------------------------------------
Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH
Sounds like a great feature to me. We regularly get authors and editors
telling us that they've lost the last hour of work by accidentally
navigating away, and I've even just had one of them email me about it a
few minutes ago, so it'd be great if you could put something together
for this.
Cheers
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC
Sounds great to me, too. I often have several tabs open doing stuff and
it's easy to lose my place and leave a page with unfinished edits.
Would this alert you when you're going to close an unsaved tab/window
as well?
Michelle
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles
Great for us multitaskers that rush and click the wrong thing
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:36:52 +0000 : chx
Very strong -1 as this is IE only.
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium
Konqueror already does this, +1 for the idea in general.
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:07:21 +0000 : webchick
Yeah, I think chx is correc that onbeforeunload is an extension
developed by MS [1] (it's not in the ECMAScript specification [2].
However, I found out that this is implemented in Mozilla since 1.7 [3]
(which corresponds to Firefox 1.0, I think?) and I've tested
Blogger.com's version of this functionality in:
Firefox 1.0.6 (working)
Firefox Deer Park Alpha 1 (working)
Opera 8.0.2 (working)
Safari 1.3 (does NOT work)
Safari 2.0.1 (working)
So it seems to be a 'de facto' standard if nothing else.
[1]
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeun…
[2]
http://www.ecma-international.org/publications/standards/Ecma-262.htm
[3] http://www.mozilla.org/status/2004-03-01.html
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:09:15 +0000 : webchick
Also, I should clarify... that "does NOT work" in Safari doesn't display
any errors or anything, it just doesn't save the form results as
expected (so pre-Tiger Safari users are going to be used to this). So I
don't see any harm in including this functionality since it seems to
degrade gracefully in non-supporting browsers.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes)
And a patch is ready! Code was based on many example, including this [4]
and this [5] along with adding my own thoughts and what not ;)
[4]
http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?O…
[5] http://www.blogger.com/app/scripts/formcheck.js
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB)
And here is the JS to throw in misc/.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_0.js (1.61 KB)
Better JS file attached without tabs. Also, tested and working great in
FF and IE6. Doesn't work in Opera 8, however no errors are present, I
believe this is just because the event handler is not supported. If
there is a work around for Opera, please let me know!
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB)
One more try with those tabs.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman
hmmm. i usually find these prompts annoying. any chance we can attach
the behavior only to the node and comment forms? those are the "high
risk" areas. what do others think?
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:42:39 +0000 : m3avrck
Yes, the JS is only loaded on the node edit/create pages, won't affect
any admin/login/etc form. Also, the prompt is unobstrusive and if you
goto save a node you don't get a warning that the contents have changed
because you are explicity saving the node (hence no reason!).
1
0
[drupal-devel] [bug] theme_image() doesn't actually output width and height of image
by Robrecht Jacques 12 Sep '05
by Robrecht Jacques 12 Sep '05
12 Sep '05
Issue status update for
http://drupal.org/node/30935
Post a follow up:
http://drupal.org/project/comments/add/30935
Project: Drupal
Version: cvs
Component: theme system
Category: bug reports
Priority: normal
Assigned to: Anonymous
Reported by: m3avrck
Updated by: Robrecht Jacques
Status: patch (ready to be committed)
This is correct, the theme('image') for the screenshots need to have
TRUE as last parameter, or omit the parameter (like the patch does).
This last patch is ok. Didn't test it, but I'm sure it is correct.
Without the width and height is will work too (it has before), but this
is cleaner HTML.
Patch is ready to commit.
Robrecht Jacques
Previous comments:
------------------------------------------------------------------------
Sun, 11 Sep 2005 16:22:28 +0000 : m3avrck
Function theme_image() doesn't actually return a width and height for an
image like it claims to do.
------------------------------------------------------------------------
Sun, 11 Sep 2005 19:30:35 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_11.patch (2.74 KB)
Ok patched attached, which fixes this issue. Also, included a patch for
system.module which sets the screen shots to 'TRUE' so image dimensions
will also be outputted there as well (which they should be!).
------------------------------------------------------------------------
Sun, 11 Sep 2005 19:33:41 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_12.patch (2.75 KB)
Fixed a tab issue.
------------------------------------------------------------------------
Sun, 11 Sep 2005 19:35:46 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_13.patch (2.75 KB)
Fixed a spacing issue.
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:18:32 +0000 : Souvent22
Used the patch, and did a quick test. Worked well for me. +1.
------------------------------------------------------------------------
Mon, 12 Sep 2005 16:21:16 +0000 : Robrecht Jacques
I don't see why this patch is needed, "theme_image" returns a img tag
with the width and height set if $getsize = TRUE.
Eg:
$node->body = theme('image', file_create_path('druplicon.png'), 'no
alt', 'no title', array(), TRUE) .
theme('image',
file_create_path('druplicon.png'), 'no alt', 'no title', array(),
FALSE)
will return:
<img src="files/druplicon.png" alt="no alt" title="no title" width="88"
height="100" />
<img src="files/druplicon.png" alt="no alt" title="no title" />
(if druplicon.png is copied to the files/ directory).
I don't see what you are fixing...
You are right about the use of theme('image') in system.module though.
The "false" should be "true".
------------------------------------------------------------------------
Mon, 12 Sep 2005 16:40:56 +0000 : m3avrck
Well the actual code in theme_image() returned this:
return '<img src="'. check_url($path) .'" alt="'. check_plain($alt) .'"
title="'. check_plain($title) .'" '. $image_attributes . $attributes
.'/>';
There is *no* mention of the $width and $height variables that are
assigned above, not used at all. I checked images in the themes
directory and none had this information, unless I was missing something
obvious, I see no way that is being generated... nothing in the above
img src about it.
------------------------------------------------------------------------
Mon, 12 Sep 2005 18:41:25 +0000 : Dries
Taken from http://php.net/getimagesize: "Index 3 is a text string with
the correct height="yyy" width="xxx" string that can be used directly
in an IMG tag.". $image_attributes contains this information. Marking
this fixed. Please reopen if not.
------------------------------------------------------------------------
Mon, 12 Sep 2005 18:49:33 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/system.module_10.patch (1.53 KB)
Dries, great catch! What prompted this originally was that the
screenshots on the theme page didn't have dimensions, didn't realize
that index [3] returned this. Anyways, this patch fixes the screen
shots and adds widths/heights.
1
0