phpfreechat 1.0 Final is out. Although I can live with beta 9.0 but some nasty bugs are really annoying. I would really love to see this module to be updated for the final. I can help regarding this task.

CommentFileSizeAuthor
#17 phpfreechat_1.patch5.43 KBowahab
#8 phpfreechat_0.patch8.48 KBowen barton

Comments

owen barton’s picture

Stéphane, the author of phpfreechat has been working on an upgrade of the Drupal module. I am currently reviewing this and hope to commit it soon.

igorik’s picture

great news! good luck for both of you and have a nice day.

Igorik
somvprahe.sk

budda’s picture

great news on the final 1.0 of phpfreechat.
Will the 5.x module be updated as well as the 4.7 one?

hectorplus’s picture

Interested in testing 5.x as well.

budda’s picture

@grugnog - any progress on being able to test out the new module on drupa l5.x - we've got a ready made community waiting to talk to each other on something other than FlashChat.

GoofyX’s picture

Hi all,

We would really appreciate any progress report regarding this issue. And surely the module is a must for 5.x installations.

Thanks.

owahab’s picture

Assigned: Unassigned » owahab

I am now working on support for 1.0 final for Drupal 5. As soon as this is done, I will do the same for 4.7.x.

owen barton’s picture

StatusFileSize
new8.48 KB

Hi All,

Here is the patch that Stéphane was working on (updated for Omar's commits to the D5 branch). Stéphane and I discussed the patch quite a bit, and confirmed that there were two problems that needed resolving before we can say that this is ready. I meant to try and build out some ideas to fix these issues, but haven't had a chance yet.

The two main problems:

  • The 'Drupal root' problem: because of security concerns the patch now stores some parameters in sessions, rather than passing them through the querystring. However, because Drupal sessions are in the database it needs to do a (limited) Drupal bootstrap to access them. Right now the patch just does chdir(dirname(__FILE__).'/../..'); to get back up to index.php (you can't easily include it from another directory, because it uses relative include declarations itself). The issue is that won't work with (probably the majority) of people who put modules in sites/sitename/modules, or even sites/sitename/modules/contrib or whatever. Also, symlinks are commonly used, which can confuse this further. The fix for this is easy - look for handler.php in the Drupal root and send the browser there if it exists (i.e. the site admin has copied it there) - this deals with the symlink issue. If the file is not there (for newbie installers) walk up the directory tree level by level until index.php is found, and then use that.
  • The second problem is more fundamental to the use of sessions. Because this requires a Drupal bootstrap and database connection there is a potentially serious performance issue with this (remember that handler.php is called every few seconds by each chatter, so the number of hits if you have a lot of chatters is very significant). At the very least we need to benchmark with and without sessions to see what the hit of this change is. If it is significant then we have a few options to try: (a) Somehow get the parameters into PHPs file session engine (rather than the Drupal database), which is a lot more lightweight, (b) Write the parameters to a file on the server with a long random name, and pass in the name of the file as a querystring parameter (i.e. replicate a very simple sessions engine) or (c) modify the phpfreechat script itself so that these parameters are left at the default if not passed in during a request. Option (c) means that we won't be able to modify these parameters from the server (e.g. making a new admin) after the chat instantiation until we also implement (a) or (b) or something similar - however we are not modifying parameters post instantiation now, and (c) would at least buy us some time to figure this out!
moshe weitzman’s picture

1) one example is _drush_locate_root() in drush module

2) drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION) is not that heavy and the workarounds are pretty nasty. i recommend keeping it until it fails us ... if you really want to play around with file based sessions, you might be able to change to files handling if you include this call before session_start(): ini_set('session.save_handler', 'files')

owen barton’s picture

Thanks for your input Moshe!

1) drush is what I was thinking of here too.

2) I agree that the session bootstrap is probably the best bet for the time being, at least so we get a release out the door - it should certainly suffice for small sites/chats. Bigger sites with lots of chatters could perhaps go the memcached route (which includes a session storage driver IIRC) and should be extremely fast!

owahab’s picture

Guys,
1) Why not *require* users to specify the location where the script is located on the hard disk. Or am I missing something here?

2) I'd go for drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION) for sure.

owahab’s picture

Version: 4.7.x-1.x-dev » 5.x-1.x-dev
Status: Active » Needs review

Please grab a recent copy from CVS and test.

hectorplus’s picture

I have been trying to make this work with no success.

The chat never loads. I have applied the patch above, same results. I have cleanURL enabled, running drupal 5.3 I have the latest chat module.

Has someone got it working?

owen barton’s picture

Version: 5.x-1.x-dev » 4.7.x-1.x-dev
Status: Needs review » Active

I just took a look at your latest commit and it does not make a whole lot of sense to me. Firstly, I think that dropping handler.php and using a phpfreechat_handler() function instead is going to kill server performance. Your approach is the one I took when I first wrote the module, because the code is obviously much simpler that way, but my benchmarking (admittedly a year or so ago) shows that it is clearly a non-starter - basically you are inviting a DDOS of your server.

- With handler.php do Drupal code is loaded at all, and no database connection at all is established. Control is passed to phpfreechat, which does a very quick check of a file to see if there are new chats.

- This would change if we used the session in handler.php, as proposed, because we would need a minimal Drupal bootstrap to load the session variables.

- Your approach, however means that a full (uncached) Drupal bootstrap is required for every single AJAX refresh. This means loading all the module code, doing a bunch of additional queries and so on - a massive amount more work. You call drupal_bootstrap(DRUPAL_BOOTSTRAP_SESSION); from the code - but this does not make any sense, because the full Drupal bootstrap has already been completed by this point (drupal_bootstrap() is designed to be run before Drupal has been loaded). This might be acceptable for a very very limited alpha release, but I would be worried about suggesting that people run this on a production site.

Remember that each chatter hits the server with and AJAX refresh every 2-5 seconds or so (depending on the admin settings). Multiply this by 50 chatters, and 10 hits per-second (in addition to the regular site load, of course) and a typical Drupal site is enough to cripple a lot of shared (even VPS) hosting environments. In my view we should be able to support at least 200 or so simultaneous chatters on a low-spec dedicated server (something similar to most developers machines), without the sysadmin needing to go to great lengths (such as installing memcached etc). Obviously bigger sites can afford the complexity, but I think we need to make the module accessible if it is going to be used.

Feel free to test this yourself - perhaps I am wrong here! :)

If my reasoning is correct, then there are several routes we could go. The simplest is to add session support to handler.php (as proposed by Stephane and myself) - this has some performance penalty of course, but nothing like a full bootstrap, and would at least let us get a release out the door for people to start testing.

Other possibilities include somehow saving to a regular files session, replicating a simple sessions handler ourselves, or providing people with a snippet of code for settings.php that somehow knows what to do.

Anyway, thanks for working on this! Let's keep chugging away and see what we come up with. It might be a good idea to post patches here, so that we can discuss first, and then commit once we have some consensus :)

owen barton’s picture

Version: 4.7.x-1.x-dev » 5.x-1.x-dev
Status: Active » Needs work

Ho hum...

owahab’s picture

Owen,

Thanks for the detailed critique, it's really helpful.

I believe most of what you said is correct, putting a bootstrap in handler.php is something I realized *after* committing the code.
But let me explain this: the module before yesterday's commits wasn't doing an integration, it was instead wrapping phpfreechat in a Drupal style. With bootstrap only we can make sure we will be able to have some real integration, I've went somehow through other ideas like mimicking php session engine but now I think only bootstrap is the way to go, so let's go for putting it in handler.php instead of a phpfreechat_handler().

I must be frank, there's still long way to go before the current code is mature enough for a release, yet I think that other modifications make sense: settings now load fine, log creation/deletion.

I will soon post a patch as soon as I have something that's working and tested.

owahab’s picture

StatusFileSize
new5.43 KB

Here's a patch that re-adds handler.php.
Note that base_url *should* be set for this patch to work.

owahab’s picture

Owen,
Let me know what you think about it.

owahab’s picture

Status: Needs work » Needs review

Having plenty of time to work on phpfreechat, I updated the last patch and tested for the past 48 hours.
I also committed the code to CVS since I need more people to be able to test.
Still waiting feedback before releasing the module for 5.x.

nishitdas’s picture

I tried the DEV version but it seems not to be working properly.
I enabled it in one node and after clicking the submit button a blank white page appeared. After disabling the chat I could see the node again

igorik’s picture

I had the same problem - white blank page (but I tested a owahab's version without patch (a week ago))
on drupal 5.2

hectorplus’s picture

Version: 5.x-1.x-dev » 5.x-1.0-beta

the chat keep loading... tested using beta and with the latest patch. The "phpfreechat" in files is not created anymore, shouldn't it be there?

igorik’s picture

with latest owahab version of phpfreechat module problem with blank page is solved. Phpfreechat is loaded.

however, there is problem that no default rooms are created (node default room, node type default roomgot or global default room)
I got just one room with name "Room".

Igor

owahab’s picture

hectr: with the beta version there's no need for the patch.
igorik: can you please post a separate bug about default rooms?

hectorplus’s picture

Version: 5.x-1.0-beta » 5.x-1.x-dev

Well, it appears that i have it working now. I created a new content of chat only then went ahead to create a chatroom, it not give me major problems. The chat looks neat in Firefox, in IE, the gray background flickers, appears and disappears and the edges are not aligned well, may be there is a conflict with the roundcorner module and the dhtml menu enabled, not sure. I have not figured out how to have users create chatrooms jet. For now it's fine.

I have D5.3 phpfrechat 1.0 phpfrechat dev modul installed.

Krotty’s picture

Title: Update module for phpfreechat 1.0 Final » Don't work for me...

If I try access to node with chat enabled - I got - 'data_public_path' parameter must be a charatere string
Clean install - Drupal 5.3, latest phpfreechat-5.x-1.x-dev, phpfreechat-1.0-final on Apache 2.2.x on Windows...
Autodetection of private and public folders don't work, in root drupal appear folders "0", "cache", "chat".
List themes on phpfreechat settings page is empty too.

If I setup adv. settings for private and public folders I got "Chat loading ... Please wait" with phpfreechat logo and stop image below... http://img402.imageshack.us/img402/3766/stopcc3.gif

Can anyone help me?

owahab’s picture

Title: Don't work for me... » Update module for phpfreechat 1.0 Final

hectr: I fixed a major bug in the parameters and -dev is the most recent code now.
If it's working for you then it's supposed to work with everyone else.

hectorplus’s picture

Krotty, the reaon you are getting 'data_public_path' is because the files/phpfreechat is either empty or the needed folders have not been created, these should have been done by phpfreechat automatically(i could be wrong). In the phpfreechat folders, there should be a 'data' folder. If it is not, i am not sure how i got my there, but i think after trial and testing, they ended up there.

Thanksk for the update owahab.

zottmann’s picture

Hi!

I've got the same problem ... Did you manage to solve it?

Regards,
Carlos.

nishitdas’s picture

@zottmann
I manually created the folders with write permission and the issue is fixed

permutations’s picture

Status: Needs review » Closed (fixed)

I'm going through the open issues since I just uploaded 5.x-1.2, and found this very interesting thread. I actually found and fixed a small bug in handler.php, but one that most people would never encounter because it was on an "if" branch that would rarely execute since most people specify where Drupal lives.

Anyway, the problems have been solved - 5.x-1.2 works well with phpFreeChat 1.2. I was able to fix this but I couldn't have written it so thanks to everybody to contributed!!