#1
News 1.44, Message for 5vision
13.12.2005 18:18 @5vision,
First excuse me for speaking in english but french and english are the only language I'm able to speak.
I had many difficulties to find you. After I received your email with the link to the archive, I tryed to write you but each time I received an error message.
Fortunately, I have some friends better than me for talking other languages
Well, I spent (really) many time to study your changes and thank you for all.
I must say that I did not used many of them.
I really hope that you will not be shocked but there are some reasons for this.
My goal is not to hurt you, I'm just trying to do my best for the modue and for its users.
Last weekend, Marcan send me some modifications he made on the module, compare to you, beleive me, I've included less things.
As I wrote in the article of the last version of the News module, I don't want to change too many things (except bugs) before a 2.0 version.
For this moment I just want a stable release.
I hope that I did not missed some important changes you made, if that's the case, tell me.
ok, let's go, here is a complete report of your changes and of mine.
1) About your file /xoops/modules/news/fpdf/update.php.
Inside it there are some functions not related at all with fpdf.
2) The function
check_and_add_domain() you have added everywhere and in every script, correct me if I'm wrong but it exists because during a time there was a "problem" when you was installing Xoops 2.2 no ?
With the first versions of Xoops 2.2, during the install process, in the "XOOPS
Virtual Path (URL)" textbox, the host was missing in the value, you just had,
example.com and not
http://www.example.com.
If that's the case, this was corrected in the last 2.2.3 version and I don't think that a module has to correct some bugs related to the core.
In the "worst" case, users can correct this value in the file "mainfile.php".
For information, the function "getenv('HTTP_HOST')" you used, will not return
anything on many IIS installations (I have verifyed it with IIS 4, IIS 5.0 and IIS 5.1). So finally, I did not decided to include and use it.
3) You have added some constant defines in some translations (english & german).
That' all right but as you don't have (and probably can't) modified all the other translations (norwegian, Traditional Chinese, farsi, francais, italian, portuguesebr, spanish, polish and dutch) when the users of those language will upgrade to your modified version, they will see a Php constant name instead of a translation (this will generate forums posts on local web site supports "why
can I see this strange ...").
4) In the file print.php, you forgotted to add a modification I have made for the meta description (use of the strip_tags() function)
5) In the file xoops_version.php, you have replaced the fourth template "news_block_top.html" with "news_block_recent.html" (this was a comment I have left in the code). That's a mistake I have made in a previous version.
It breaks the compatiblity with Xoops 2.0 users and it "correct" a problem related to Xoops 2.2.x
You have added a new block, "news feed", that's nice but like in the point #3, this will cause some translations problems. I will add many things in the 2.0 version of the News module, like this and many new translations, but no yet, sorry.
Understand me, I'm also a translator and I know how boring this job is. I'm not sure to always have the correct translations and I prefer to disturb translators one time for some major changes instead of disturbing them every 4
mornings for some "minor" changes
6) File /xoops/modules/admin/functions.php.
You have replaced :
Zitat:
with :
Zitat:
in fact you have replaced the single quotes with double quotes.
There's no problems at all with the code I wrote.
In the xhtml specification you can use single and double quotes for tags parameters.
For example, you can (successfully) test this code with the w3c validator :
Zitat:
7) The file /xoops/modules/admin/index.php
a) This time, I had problems with double and simple quotes.
You have made some changed but missed the really important bugs
The correct line for the collapsable bar should be :
Zitat:
echo "<img onclick=\"toggle('toptable'); toggleIcon('toptableicon');\" id='topnewsubicon' name='topnewsubicon' src='" . XOOPS_URL . "/modules/news/images/close12.gif' alt='' /></a> "._AM_NEWSUB."</h4>";
Thank you for pointing me this problem.
I have corrected the code of all the collapsable bars.
- In the function LastStories(), you forgotted a correction I've previsouly made in the 1.43 version.
Before :
Zitat:
echo "<form action='index.php' method='post'>" . _AM_STORYID . " <input type='text' name='storyid' size='10' />
after my previous correction :
Zitat:
echo "<form action='index.php' method='get'>" . _AM_STORYID . " <input type='text' name='storyid' size='10' />
Same thing in the funcion named expStories()
b) In the Newsletter() function.
- You have made what you called a fix.
In fact you have added an additional "parameter" to the filename used to create
the newsletter file.
May be we can consider this as an additional security measure (but in no case a "fix") but when the file is created by the script, user can see two links, the first to download the file and the second to remove it so it is really necessary ?
- You have found a bug in the code used to create the final timestamp.
If you create a news today and want to export it, it will not be possible with my code. Thank you for the correction, it's included.
- Later in this function you forgotted to modify the code as it was made in the 1.43 version :
Zitat:
$search_pattern=array('%title%','%uname%','%created%','%published%','%expired%','%hometext%','%bodytext%','%description%','%keywords%','%reads%','%topicid%','%topic_title%','%comments%','%rating%','%votes%','%publisher%','%publisher_id%','%link%');
$replace_pattern=array($onestory->title(),$onestory->uname(),formatTimestamp($onestory->created(),$dateformat),formatTimestamp($onestory->published(),$dateformat),formatTimestamp($onestory->expired(),$dateformat),$onestory->hometext(),$onestory->bodytext(),$onestory->description(),$onestory->keywords(),$onestory->counter(),$onestory->topicid(),$onestory->topic_title(),$onestory->comments(),$onestory->rating(),$onestory->votes(),$onestory->uname(),$onestory->uid(),XOOPS_URL.'/modules/news/article.php?storyid='.$onestory->storyid());
$content = str_replace($search_pattern, $replace_pattern, $content);
c) In the LaunchExport() function
About the filename, like in the previous point, I disagree, this is not a fix but an additional security measure where the delete link can play its role.
So I will keep the original filename I've made.
But I have reported the correction of the final timestamp you've made in the previous point. Thanks again for it.
You replaced <xoops_stories> with <news_stories> and <xoops_topic> with
<news_topic>, yes why not...
Your correction about the utf8 encoding is not complety equal to mine.
I saw your function called
back_encode_xml_data.
Personnally I have just added a xoops_utf8_encode($content) before to write to
the file.
Concerning the timestamp modification you made (you have formated it), I prefer to keep the "original" timestamp, inside an export file I find it more useful, it take all its sense when you use the file to reimport it on your database.
8) /xoops/modules/news/fpdf/chinese.php.
You forgotted some corrections made in News 1.43 like this one :
Zitat:
if (!defined('XOOPS_ROOT_PATH')) {
die("XOOPS root path not defined");
}
That's the same for the files ex.php, fpdf.php, gif.php, japanese.php, korean.php.
You have added a missing index.html in the different folders, I have made it too.
9) In the images folder.
I sav your new pictures. Probably like you, I find the actual pictures horrible but if actual users of the module see their picture change, may be they will not agree
10) /xoops/modules/news/include.php.
In the function getmoduleoption(), you forgotted to add something I made in a previous version, a default value for the
$repmodule parameter.
11) /xoops/modules/news/templates/news_article.html
This is not a fix as you wrote but a personnal choice you have made.
You choosed to open printable versions (of articles) in a new window.
I will not change the template's behaviour for this moment.
12) /xoops/modules/news/templates/news_archive.html and /xoops/modules/news/archive.php
I have reported the
border='0' and the missing
</a> tag.
I did not added the pdf picture and link.
That's a good idea but it will added in the module version 2.
13) The file /xoops/modules/news/backendt.php
First I don't want to include the module's version.
In a previous version I have, for security reasons, removed the Xoops version so I will not add this.
You have replaced the "channel_category" with the News topic's title, that's a good idea, I have added it (but not the way you have done it)
Note, you are creating an object with this code while it's already an object :
Zitat:
$article = new NewsStory($story->storyid());
and you forgotted some modifications already made to correct some problems with
Php 5.0.5.
14) archive.php
You have placed a comment around this text :
Zitat:
######################################################################
# Original version:
# [11-may-2001] Kenneth Lee -http://www.nexgear.com/
######################################################################
It's not necesary. In Php there are 3 ways to create a comment :
/ * * /
//
#
15) article.php
You forgotted a previous correction, line 449
Zitat:
$xoopsTpl->assign('xoops_meta_description', strip_tags($article->title()));
16) all the language files in fpdf/language folder.
I have made some of the modifications you made.
17) About your changes to be able to use local pictures in PDFs.
That's excellent and very interesting.
I still have to study it, like it is actually and for what I saw, it will only run if the curl extension is runing. Plus, it also uses the call to getenv('HTTP_HOST') wich does not returns anything under IIS.
In my opinion this will not run on many systems.
I don't prefer to add it for this moment because if I write that now, PDFs runs with local pictures while it's not always true, I will have to justify it. Sorry.
Finally, a new version is ready, it includes some of your modifications and Marcan's modifications too.
Thanks again for your help !
Bye,
Hervé
