Hi

It would be great to have facebook like chat on bottom panel, in this case the best solution will be probably to integrate it with AppBar module http://drupal.org/project/appbar

for working facebook like chat in drupal (with own bottom panel) and maybe some inspiration you can try Bowob module: http://drupal.org/project/bowob

thanks
Igor

CommentFileSizeAuthor
#40 uchat.zip6.83 KBthomas4019
#37 dchat.zip5.75 KBthomas4019
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IceCreamYou’s picture

Hi, I'm the Appbar maintainer. Appbar's API makes integration relatively straightforward. I don't have time to actually write a patch any time soon but I will be happy to help if this gets taken up and there are any questions.

Anonymous’s picture

igorik, thanks for the suggestion. do you have the time/skills to work on this?

IceCreamYou, thanks. how does Appbar handle page reloads?

igorik’s picture

Hi Justin,

Unfortunately I have no skills for something like this. I am trying to help community with my comments, ideas, bugreports and future requests and testing for various modules.

Have a nice day
Igor

IceCreamYou’s picture

Title: facebook like chat + integration with appBar » Facebook-style chat via integration with Appbar

Justin - I'm not sure what you mean. I've tried to keep the module optimized and efficient, if that's what you're asking.

Anonymous’s picture

here's a unpolished patch, just me playing with the API. looks very promising to me, i'd like to explore more.

@IceCreamYou - first issue i hit with the API i'd like to discuss - seems i can't add ids to appbar regions at runtime? the use case here is to have a user choose arbitrary chats that they want to keep open in the appbar, something like this:

function chatroom_appbar_regions_alter(&$regions) {
  foreach (chatroom_chat_get_appbar_chats($user) as $node) {
    $regions['chatroom-chat-appbar-' . $node->nid] = theme('chatroom_chat_appbar_interface', $node);
  }
}

it doesn't look like this is supported, as this code in theme_appbar_bar():

    $collected = module_invoke_all('appbar_region');
    $ardefault = variable_get('appbar_regions', drupal_map_assoc(array_keys($collected)));
    drupal_alter('appbar_regions', $collected);
    foreach ($collected as $name => $region) {
      if ($ardefault[$name] && $region) {
        $regions .= '<div id="'. $name .'">'. $region .'</div>';
      }
    }

throws warnings if the $name key doesn't exist in $ardefault. i could mess with the 'appbar_regions' variable, but that feels like a layering violation. am i missing something?

would you accept a patch that allows for dynamic runtime keys for appbar_regions?

Anonymous’s picture

Assigned: Unassigned »
Status: Active » Needs review
IceCreamYou’s picture

I don't quite understand what you mean by dynamic regions (I mean, I understand the concept, just not how it doesn't match up with the existing code). However, I'm certainly willing to accept a patch, as you're probably right and I'm just overlooking something.

Also, it looks like the patch you refer to in #5 didn't get attached...

Anonymous’s picture

Status: Needs review » Active

right, sorry, probably not a useful patch right now anyway. just trying to formulate a reply has helped me figure out what i was missing.

hook_appbar_region is called per-request, and not cached anywhere, so i just have to make the return dynamic and per user, and we're good.

IceCreamYou’s picture

Okay, well at least now I know not to cache hook_appbar_region() in the future, heh. Of course let me know if anything else comes up that I could help with.

BetaTheta’s picture

subscribing

IceCreamYou’s picture

Issue tags: +Appbar

Tagging to make this easier to find.

robotjox’s picture

subscribing

lintlin’s picture

subscribing

Tunox’s picture

subscribing

zuzu83’s picture

subscribing

Styx-1’s picture

subscribing

Laza’s picture

subscribing

ragavendra_bn’s picture

subscribing...........

thomas4019’s picture

subscribing

nonom’s picture

subscribing

Anonymous’s picture

subscribing....

only joking. i'll put up some code for this shortly, stay tuned. @all the subscribers: who can help out with code?

IceCreamYou’s picture

Sweet. I'm overloaded right now so I can't actually help with writing the code (for another week, at least) but I will be happy to review any patches.

mstrelan’s picture

subscribe and can potentially help with some code

Anonymous’s picture

mstrelan: great thanks! i'm beejeebus in the #drupal IRC channel, feel free to drop by there so we can chat.

MWoertler’s picture

subscribing

AlanAtLarge’s picture

subscribing

jrivelli’s picture

Subscribing

jacobpov’s picture

Subscribing

hiker1989’s picture

subscribed.......

thomas4019’s picture

I have started working on a chat module based on the suggestions above, please contact me with any more suggestions. Currently I have don't think it will be able to support anonymous users chatting as the AppBar is only for logged in users.

I hope to have a working development release within a week.

hiker1989’s picture

I would suggest that the users should be able to chat with all users as well as with each other...............

jrivelli’s picture

I would suggest that you make it friends only with a flag friend or UR integration

thomas4019’s picture

hiker1989, I don't really get what you mean, do you mean you think authenticated users you also be able to chat with anonymous users. I haven't done much with anonymous users chatting yet. Currently it shows a list of logged in users, you can click their name, and start chatting with them.

jrivelli, a fantastic idea, this would make it even more like gmail/facebook chat. This feature will really help on large scale sites. I will try to implement it as well.

Currently each user chatting takes up one connection on the server. So a server will be limited on the number of concurrent chats by the server configuration. The chatting is going to be server<->database bandwidth heavy but not very client<->server heavy. I'm still a few days out from a stable release.

Anonymous’s picture

Title: Facebook-style chat via integration with Appbar » new chat module with Facebook-style integration with Appbar
Project: Chatroom » Application Toolbar (Appbar)
Version: 6.x-2.x-dev » 6.x-1.x-dev
Assigned: » Unassigned

thomas4019: great news! it sounds like you're developing a completely new module? so its probably best to move this issue out of the chatroom queue, and rename the issue to reflect what you are doing if that's the case?

hiker1989’s picture

Well what u just said is what i meant!! Being HONEST u r really doing a great job!! I tried myself to hang on for a chat module but .........................................

IceCreamYou’s picture

Sounds like it will be an independent module really, so I guess the Appbar queue works until it can appear on d.o. Looking forward to seeing what you have. Even if it's not done I will try to get some time to look through the code and offer suggestions if you post it.

thomas4019’s picture

FileSize
5.75 KB

Here's the code I have so far. I would love any feedback and suggestions. I know I probably need to user the theme() function more and that's next on my list. I plan to apply for Drupal CVS access soon and will use this a my work for the application, so if you have any sugestions specifically related to getting it ready for that as well that would be awesome. The module is named "Drupal Chat" currently, but I think I'm going to change it to "User Chat" soon. This still needs some serious work. This module requires no special setup. It creates a permission "ability to dchat" which if enabled causes the chat region in the Appbar to appear.

Thanks,
Thomas Hansen

IceCreamYou’s picture

I haven't tried the module but here are my comments from looking at the code:

The Drupal Coding Standards say that code blocks should be indented 2 spaces, not 4. And you don't have to indent the top-level code block (e.g. no need for spaces before the "function" keyword).

All of your functions need documentation per the Doxygen formatting conventions. In most cases that just means a docblock with the words Implementation of hook_whatever(). in it. You should also have an @file block although that's not really important.

The $Id$ at the top of the file should contain only // $Id$ -- the Drupal CVS repository has a script that fills in the rest automatically, but it can't do that if you've already filled it in.

You mix single and double quotes often around strings. Typically you should always use single quotes except around SQL queries. Also, no need to put numbers in quotes. PHP literally interprets strings containing only numbers as numbers (not quite true, but that's the effect).

It's a good idea to put hook_menu() at the top because it is usually a way to quickly learn what the module provides. That's not so much the case here but maybe putting hook_appbar_region() at the top would be good too.

Basically, you should install the Coder module and see what it says -- it's a great way to catch stylistic mistakes. ;-)

Don't add your CSS and JavaScript in hook_init() since that is called on every page load regardless of whether the user has permission to chat and whether the chat is even loaded. Instead, add it in hook_appbar_region() and your page callbacks.

A query like this:

$set = '('.$other_uid.', '.$user->uid.')';
$query = "SELECT data,dchat_chats.from FROM {dchat_chats} where dchat_chats.from IN %s AND dchat_chats.to  IN %s ORDER BY dchat_chats.timestamp ASC";
$result = db_query($query, $set, $set);

...should actually be this:

$query = "SELECT data, dchat_chats.from FROM {dchat_chats} where dchat_chats.from IN (%d, %d) AND dchat_chats.to IN (%d, %d) ORDER BY dchat_chats.timestamp ASC";
$result = db_query($query, $other_uid, $user->uid, $other_uid, $user->uid);

...because of where the data comes from, that's not an SQL injection risk, but you should never insert values directly into a query without using Drupal's substitution tokens.

When you use watchdog(), $type is typically the module name (not e.g. SQL) and $message is supposed to be descriptive (not the SQL query). If that's just in there to debug, you're better off using drupal_set_message() or installing Devel and using dsm().

You have a XSS vulnerability in lines like this (dchat.module lines 44, 56, etc.):

$out .= '<div><span class="dchat_chat_username">' . $usermap[$uid] . ':</span> ' . $row['data'] . '</div>';

...you need to use check_plain() to sanitize user input (both the username and the message, although I would recommend doing theme('username', $account) for the username instead). Don't check_plain() before you store the data (like line 99); Drupal's model is to store the exact data on input and filter on output.

dchat_chat_update_status() does absolutely nothing and will always return a blank string.

intval() on line 98 is redundant.

Instead of initializing $row as (object)NULL, just create $row as an array. drupal_write_record() will then automatically convert it to an object properly.

Lines 107-114 are redundant.

You're using t() incorrectly on line 133, it should be:

t('Number of minutes for chat messages to remain in database. Number of chat messages currently in database = @count', array('@count' => $msg_count))

None of your settings have titles!

The function dchat_admin_settings_validate() is automatically assumed to be the validation function, no need to add your own manually. Also you should check that the numbers entered are positive integers.

I have to go, I'm leaving for the weekend. That's as far as I was able to get but hopefully it will help.

thomas4019’s picture

Wow, thanks for all your invaluable help. Oddly enough a lot of those peculiarities came from me borrowing code snippets from other modules (especially Nice Menus). I've fixed almost all your suggestions except a few. I tried adding the CSS/Javascript in hook_appbar_region and it simply didn't work so I have left it in hook_init. Yeah, the watchdog was just for debugging; I have no idea why I didn't use dpm(). I haven't implemented a theme function yet, because most of the HTML is formed on the client side using java script. I can't find any documentation on creating Javascript theme functions other than that they exist. Also, when I removed the $form['#validate'][] = 'uchat_settings_validate'; the validation didn't work so I have left it in.

thomas4019’s picture

FileSize
6.83 KB

Oops, I forgot to attach the code. Here it is. I would love any more suggestions and feedback.

IceCreamYou’s picture

I'm glad that was helpful. Hopefully I'll get a chance to review the rest of the module soon.

I can't find any documentation on creating Javascript theme functions other than that they exist.

Yeah, Drupal's JavaScript library is very poorly documented. I don't really know anything about it either. Most implementations I've seen do the theming on the server and then pull it in via AJAX.

Also, when I removed the $form['#validate'][] = 'uchat_settings_validate'; the validation didn't work so I have left it in.

Well you can't just assume that Drupal will figure out your validation function magically, it has to be named the right way. If the function that generates your $form array is mymodule_my_form_function(), then Drupal automatically assumes your validation function is mymodule_my_form_function_validate() and your submit function is mymodule_my_form_function_submit(). (See the pattern? <form_id>_<action>().)

Anonymous’s picture

thomas4019 asked me this via email:

I was curious why you implemented chatroomread as a simple PHP file instead of using the built-in drupal page system by menu callbacks. I have implemented my chat through the menu callbacks so that a user doesn't have to copy a file for installation. Is there anything wrong with this?

its not about right or wrong, just trade-offs. a full drupal bootstrap for every chat polling request will eat your server very, very quickly. however, using the drupal menu system is much cleaner and simpler than a custom chat script. pick your poison.

chatroom module with caching will be much, much lighter on a server than your module. its not even close. even given that, however, any chat implementation that uses polling won't scale much anyway.

having read through the code, i'm not sure why you implemented a new module. we could easily fold what you've got (particularly the theming stuff) into chatroom module. let me know if you're interested, as you've made a bunch of cool new functionality, but also reimplemented (in a slower, more server intensive way) a lot of existing functionality.

IceCreamYou’s picture

Justin, I would still like to see integration between ChatRoom and Appbar, even if Thomas' module stands by itself and becomes its own d.o project. If you think Thomas' module is a viable base for integration, I would like to see that path moving forward.

thomas4019’s picture

I will look into building it off of Chatroom again. There were two main reasons why I decided to start a new module. The first was the mandatory copying of the PHP file for Chatroom to work; I really wanted a module to work without unusual setup. The second was that the Chatroom module appeared to be doing polling at regular intervals. With my module I really wanted to do long polling, where the client makes an AJAX request and the server delays until there is a response (http://en.wikipedia.org/wiki/Comet_%28programming%29).

Also my chat functionality seems very different from Chatroom's. Mine is really for many people doing one-one communication.

IceCreamYou’s picture

I haven't used ChatRoom in almost two years honestly, so I don't actually know what it's like now, but obviously one-to-one chatting is a critical component here. It just seems to make more sense to build on the base of an established and popular module if possible. Thomas, I will support whatever path you choose, but I'm interested in building integration with ChatRoom whether or not you choose to go that route.

Anonymous’s picture

thomas4019, IceCreamYou, thanks for the discussion.

thomas4019 - i think it would be best to roll your efforts into the chatroom module. i'm open to co-maintenance of the module, and very liberal with the commit bit. i don't see the module as my personal plaything, and would be happy to collaborate with you, particularly to get one-to-one chats and appbar integration. detailed comments below.

@long polling - if you already understand this, ok, but just to be sure. long polling works better than polling *if you've got a backend built for persistent connections*. apache, which most drupal sites run on, just isn't. if you use long polling, you tie up an apache process per chat user per chat all the time. once you've reached max clients, even if the server is not doing much, you can't serve other requests. so while the server will do less work than the polling method, it will become unavailable very quickly. a busy server that can still serve other requests is better than a server that can't serve other requests.

honestly, using a standard LAMP stack that runs drupal as the backend for chat is just the wrong decision from the start *if scalability is important to you*. for chatroom, i've explicitly traded away scalability for tight integration and ease of setup - you don't need to run an IRC service or similar, and module developers can hook into it with standard drupal methods. thomas, if you need more explanation, lets chat in #drupal.

@chatroomread.php - there are two issues, first, a full drupal bootstrap per-poll is just unacceptable performance wise, even if you're not trying to scale far. that's the reason for the existence of chatroomread.php - its much, much faster and lighter than a full drupal bootstrap. second, the chatroom module can live in any number of places on the file system, so the path from the chatroomread.php file to the drupal root *has to be sent by the client, and cannot be trusted*. there are various ways to try to validate this, but they suck, because you have to validate the path *before* you've included any other drupal code. so you can't use anything in settings.php, you can't use any drupal functions etc. as far as i can tell, there is no way to do this safely.

there's an unfinished task related to chatroomread.php setup in the chatroom issue queue, which is to fire a drupal_set_message() if the module is enabled and chatroomread.php hasn't been copied to the webroot. this would kill most of the "my chat doesn't update bugs" that come up every now and then.

@one-to-one - i'd love to see this feature in the chatroom module, so if you're willing to work on adding it, i'll help. it was lower on my list of priorities, but is a much-requested feature.

thomas4019’s picture

Well, you finally have me convinced. There really is no reason to reinvent the wheel here, and my planned module and Chat Room really would have been very similar. I now agree that with a few patches Chatroom could serve the functionality I was desiring.

I am delving into the Chatroom code now. I would greatly appreciate your help in adding this new functionality.

Anonymous’s picture

awesome! lets work out an action plan.

IceCreamYou’s picture

For what it's worth, Appbar's alert system also does periodic (i.e. not long) polling, albeit with a bootstrap. It doesn't update anywhere near as often as a chat, so it's not much of a performance issue for most sites -- but like Justin said, if you want to do this kind of thing you have to either have a server set up to handle the load or accept that there will be scalability issues on sites with thousands of concurrent connections.

(If you're interested, I do have performance optimizations I'd like to get in when I get some time, mainly based on window focus and timing inactivity.)

@long polling - if you already understand this, ok, but just to be sure. long polling works better than polling *if you've got a backend built for persistent connections*. apache, which most drupal sites run on, just isn't. if you use long polling, you tie up an apache process per chat user per chat all the time. once you've reached max clients, even if the server is not doing much, you can't serve other requests. so while the server will do less work than the polling method, it will become unavailable very quickly. a busy server that can still serve other requests is better than a server that can't serve other requests.

I have considered using Comet for Appbar's alert system in the past, and this is why I ultimately didn't. That's not quite the end of the story -- a lot of Comet systems periodically terminate the connection to free up resources -- but Apache is not designed to handle it well.

thomas4019’s picture

Here's my current ideas for the Chatroom-Appbar integration. Let me know how you think the integration should be done.

Sample Layout of Appbar
[ [Chatrooms] [Chats] [User Chat] [Alerts] ]

Each of the three chat-based regions would be optional. I'm planning on adding the "Chatrooms" and "Chats" regions first and basing them off of the two blocks that the Chatroom module already supplies. When you click on one of the chat based regions, a box would popup in the bottom left of the screen, like Gmail chat. The user chatting will be difficult as chats will have to be created and deleted dynamically. I think it would be great for the Chatroom module to provide an easy API for adding a chatroom. Something like theme(chatroom,cid) where cid is a unique identifier for the chat. Maybe there's something like this already.

I am leaning toward making a way for Appbar to be able to show any block as a region. That way out of the box a basic "Chatrooms" and "Chats" region would be possible. The blocks title would be shown as an Appbar region; Then when you click on that title, the full blocks content is shown in a popup in the bottom right. I think this could be cool for the "Who's new" and "Who's online" blocks. This would also allow a lot more built in integration.

Thomas Hansen

Anonymous’s picture

sounds good!

i'll reread the appbar code over the next week so i can be more useful helping you with the work.

when you are ready, the most useful thing for us to do would probably be to open a fresh meta-issue with a list of child issues we need to work through, so we don't end up with one huge issue, but we still have somewhere to track the effort.

also, it probably makes sense for us to create a chatroom_appbar module, and only add/refactor code in the core chatroom module that's necessary to make the integration work.

IceCreamYou’s picture

I agree with Justin.

Allowing any block to appear in the Appbar is not easy. It's not possible to add a region to a site from a module, so the interface for turning a block into an Appbar widget would have to be completely new. Probably it would need a new page with a select widget to choose a block and turn it into an Appbar region, and then it could be checked on the page where regions can be enabled. Then some complex JavaScript would need to be written to make the block's content pop up over the Appbar above the place where the region appeared on the Appbar.

For the layout of the integration, I would just have one region on the Appbar, called Chat. Clicking the Chat region would make a listing pop up above where the Chat region was. It would be divided into two section: Users (or Friends if either Flag Friend or UR was installed) and Chat Rooms (where more than one user can chat). Each section should be collapsible like Facebook does with Friend groups.

I have two very large projects coming up very soon so it is unlikely that I can spend much time writing code for this. However I will definitely put aside some time to review any code that gets written.

shadcn’s picture

Hello everyone

let me have your feedback on this please : http://drupal.org/project/tray

Thanks

ChaosD’s picture

appbar should be extended to easily add blocks like tray does. that way everyone is happy

IceCreamYou’s picture

Per #852198: Future of Tray I will allow Appbar to use blocks (I didn't realize it was possible before) and I hope that Arshad will collaborate on this instead of trying to duplicate a lot of the work I've already done.

jrivelli’s picture

If the features from the Tray module were added to appbar, that would be one awesome module!

IceCreamYou’s picture

FWIW, I have Appbar working happily with blocks now. These are the things I have left to do before releasing a new branch:

  • Add an option to the block settings to show the block content as inline instead of in a popup above the bar (I already have it working either way, there's just no setting for it)
  • Improve the styling for blocks
  • Fix Views integration with the alert system

Views needs fixing because I changed some things:

  • I got rid of the Appbar Default Alerts submodule; all alerts should now be created using Rules or Trigger, or alternatively a system like Activity can be used to generate the alerts view
  • I changed the database schema somewhat; I haven't written an upgrade path yet

There are some other things I would like to add or fix but I'll get the dev out as soon as the above things are fixed (in other words, as soon as it's stable enough) and then work on the other things.

ChaosD’s picture

very nice to see some devent progress here! much appreciated. i will start testing it as soon as you release it

lolomu22’s picture

ok. so now you're duplicating others...

IceCreamYou’s picture

Status: Active » Fixed

@lolomu22: Tray and Appbar are mutually exclusive. A site cannot have two bars at the bottom. Appbar is more mature. The right path for Tray would have been to contribute its ability to hold blocks to Appbar. Instead, you duplicated a lot of work, and I realized that hook_system_info_alter() existed. You would have benefited everyone if you had written a patch for Appbar instead of writing an entirely new module yourself.

Did I duplicate work in Tray? Well, yeah. It's not like I'm just going to give up maintaining the more mature module because someone else came along and created a new module instead of writing a patch. I would have preferred to work together, but you already declined.

Hey, it's open source.

@all:

I just created a new 2.x branch of Appbar that supports blocks (with an alpha). There is an upgrade path from the 1.x branch, but I haven't tested it. Please try it out!

As to this issue specifically: since Appbar now supports blocks, the remaining problem in allowing ChatRoom to work with Appbar is to let Chats operate in blocks. Therefore, and in accordance with what Justin suggested, I'm marking this issue "fixed," and discussion of making ChatRoom work with blocks can take place in the ChatRoom queue (although a link from here to the new issue would be nice).

For those who aren't discriminating, there are a number of Chat modules that already use blocks, and they should all work with Appbar now.

jrivelli’s picture

@icecreamyou, great work on developing and maturing this module. Just upgraded to the alpha release and the blocks chunks are working well thus far. I am debating on what chat program to use, but I'll figure it out eventually which I like best. Cheers mate!

Thumbs up on all your modules from me!

IceCreamYou’s picture

It's great to hear that jrivelli, I'm glad you're enjoying my modules and that everything is working.

hiker1989’s picture

Well i thank all of you here! I am out of a big tension only by the great efforts of you people.

But can u plz tell me if this chat uses database to maintain the sessions, won't it increase load on the website database requests.. Being honest! I am not a full-fledged web developer & sorry if this is a lame point. But can u plz explain it for me.

thomas4019’s picture

I really don't know how to integrate chatroom and the appbar module. After reviewing the chatroom's code, I don't think I'm the right person for the job. I don't know Chatroom's code well enough, and I don't have enough time to figure it all out. Also, it appears someone has already created the functionality I personally was looking for with this module, http://drupal.org/project/drupalchat.

Thomas Hansen

Status: Fixed » Closed (fixed)
Issue tags: -Appbar

Automatically closed -- issue fixed for 2 weeks with no activity.