I just pushed a change "load mnc count synchronous from cache if possible", this introduces a new method: Drupal.mnc.getCountFromCacheSafe() which is capable of detecting localstorage on it's own (borrowed it from modernizr) and read the last valid cache count cache. This method is now called within mnc.tpl.php: <script type="text/javascript">document.write(Drupal.mnc.getCountFromCacheSafe());</script>. This is done synchronous on purpose:

Current browsers do not wait with rendering till all js has done it's job, so it's not safe to modify DOM values on DOM ready alone, it will lead to (previous version of mnc: having an empty mnc container, or having to force a 0 in it) variables not beeing initialized and ready as we need them. This function bypasses this, but be aware that this also implies that mnc.js needs to beloaded in head and can't be put in the document body.

If you want to defer loading of mnc.js you will now have to modify the mnc.tpl.php and remove the call to Drupal.mnc.getCountFromCacheSafe() and replace it with something that suits your workflow , e.g. 0 or empty, if missing or changing values is not an issue for you (for me it was).

Please test and give some Feedback here.

Comments

Andre-B’s picture

Issue summary: View changes
Andre-B’s picture

Title: Feedback neeed: load mnc count synchronous from cache if possible » Feedback needed: load mnc count synchronous from cache if possible
Andre-B’s picture

Title: Feedback needed: load mnc count synchronous from cache if possible » Feedback needed: load mnc count synchronous from cache on page load if possible
Andre-B’s picture

removed the old style of the counter init now as well. If you need to have your scripts executed the old way make sure to do a Drupal.mnc.updateCountInterval(true); somewhere in your js on dom ready.

JordanMagnuson’s picture

A couple of things:

It looks like an extra closing span has been added in mnc.tpl.php:

<script type="text/javascript">document.write(Drupal.mnc.getCountFromCacheSafe());</script></span></span>

With this latest change, the unread-message-count badge span is not updated with 'has-unread-messages' or 'no-unread-messages' classes on page load, so I can't hide the message count badge if there are 0 unread messages. See screenshot: http://www.anony.ws/image/DUFW

  • Andre-B committed 727cfb9 on 7.x-1.x
    load mnc count synchronus #2460859
    
Andre-B’s picture

I just moved the creation of the mnc container and all it's contents completely to javascript (document.write), I am doing this so the browser inserts the whole structure in one block, rather than executing multiple document.writes to handle the logic. Problem with this is: it currently has no fallback, but it should be possible to put the same structure in noscript tags again.

Other opinions?

JordanMagnuson’s picture

Tested this out, and it seems to be working as intended. Just need to add <noscript></noscript> underneath with php implementation for getting unread message count and $unread_messages_indicator.

It's messy and redundant (script and noscript versions) but it works. Not sure what the best practices are for this kind of thing...

Andre-B’s picture

well, noscript is fine, but I wont display the counter there (it's not possbile without removing the cache for this block) - if someone needs to have that counter without javascript as well than he or she shall update the template file (he has to send the counter anyways with each request then).

JordanMagnuson’s picture

Yeah, displaying the counter for nojs is probably overkill.

JordanMagnuson’s picture

Discovered a problem: with this patch, unread message count is not updating until client side refresh (no update on initial page load). So if client side refresh is set to zero (or a high value), the count never gets updated. Screenshot: http://i.imgur.com/TsPwjZy.png

JordanMagnuson’s picture

[Oops: wrong thread]

Andre-B’s picture

Jordan, I am not sure that this really is a bug, if it's initial value is 0 it should be fine, also the fallback should be there (leaving it at 5 minutes or whatever suits your site will take care of any network problems that might have occured between count updates). If you don't send any information about a new count (like with nodejs or manually via Drupal.settings. how should mnc get updated.

bserem’s picture

I am getting this in the console:
Uncaught TypeError: Cannot read property 'getCountFromCacheSafe' of undefined

Not sure if these two are related.
Did I do something wrong with the installation? I skipped the example module, since our site has thousands of messages and generates new every minute. Could this be a problem?
The read and notification columns in the message sql table are NULL in all messages (even ones created after mnc got installed).

Andre-B’s picture

Failed to load resource: the server responded with a status of 404 (Not Found) http://localhost:8080/socket.io/socket.io.js
Uncaught TypeError: Cannot read property 'getCountFromCacheSafe' of undefined

That error basically states that Drupal.mnc.getCountFromCacheSafe is not available, which might be related yes. Do you load mnc before executing Drupal.mnc.getCountFromCacheSafe()?

Not sure if these two are related.
Did I do something wrong with the installation? I skipped the example module, since our site has thousands of messages and generates new every minute. Could this be a problem?

example module shouldnt be needed

The read and notification columns in the message sql table are NULL in all messages (even ones created after mnc got installed).

you will have to do a $message->notification=1; to have that column filled, the read column is not used in my setups any more (since I use flag for flagging messages read).

bserem’s picture

Thanks for the fast reply!

The first error was from node, I fixed it (and edited my comment before I saw your reply, sorry).
The second still remains.

What do you mean by "load mnc before..." ?

bserem’s picture

It just hit me! We are loading all JS at the bottom of the site, so yes, the mnc.js was probably running after the call to Drupal.mnc.getCountFromCacheSafe().

I'm back on track, thanks!

Andre-B’s picture

mns's js files need to be in the head, some modules put those files in the footer.

bserem’s picture

Great, thanks :)

JordanMagnuson’s picture

Version: » 7.x-1.x-dev
Status: Active » Closed (outdated)