This is sorta a distributive install since all blocks get ajaxed. I have a feeling most blocks are static thus the default should be an allow list. BTW I can actually use this module now, good job!

CommentFileSizeAuthor
#6 ajaxify_regions-802660.patch1.76 KBmikeytown2

Comments

csevb10’s picture

csevb10 commented

It only "ajaxes" blocks that are considered, by the system, to be uncacheable (yes, I still need to document that better...). So, it should only try and add Ajax to the handful of blocks that are set in a few of the most restrictive (to-caching) blocks. I'm not opposed to changing it, though, so if we can prove a couple cases where it makes sense, I'll change it out.

mikeytown2’s picture

mikeytown2 commented

One of the blocks I have bombed my site (for anonymous) after installing this module. Still need to look into it but the entire pages content was replaced with today's date. The block uses javascript to print the date. I'm sure other people will have other issues with this default behavior. In short I need to hack your module and change the defaults if I wanted to use this module on a live site.

csevb10’s picture

csevb10 commented

Is there an example of the block or the javascript the block utilizes so that I can recreate this issue? It'd be interesting to delve into whether this is an issue that I can resolve or if it's something that'd need to be changed by the modules that create the blocks.

Seems more like it'd be better for me to allow an enable/disable for ajaxifying blocks in general. I think hacking the module seems a bit extreme, but I can see how the replacements coming on once you enable the module (and before you're able to make changes) could be problematic. I know that one of the editors (tinymce? fck?) had a similar issue awhile back collapsing fieldsets and the way that the editor's js was printed to the screen, so I'll see if I can track down what was happening there and see if I can shed some light on the issue (if I remember correctly, that turned out to be a problem with the WAY that the editor printed JS to the screen without using JQuery functions, so it may turn out that these blocks SHOULD be able to be ajax loaded but can't until the underlying modules are changed).

mikeytown2’s picture

mikeytown2 commented

If a block has this code in it, bad thing happen

<script type="text/javascript">
  document.write((new Date()).toLocaleDateString());
</script>

User created blocks will have document.write in it. Bad things will happen in this case.

csevb10’s picture

csevb10 commented

ah, yep. document.write is the devil. pretty sure that's the exact same thing that gave problems elsewhere, too. :-)

Ok, I'll look to make turning on the module a setting in the admin so that there is no special hoop-jumping to be able to enable the module. Any desire to put together a patch, otherwise, I'll get to this as soon as I can.

As a side note, things that are admin-created are always considered uncacheable which is sorta annoying. That's really something that should be an option for end users to improve performance.

mikeytown2’s picture

mikeytown2 commented
Status: Active » Needs review
StatusFileSize
new ajaxify_regions-802660.patch1.76 KB

my editor does auto whitespace cleanup for me. Here's the patch that changes the default; may not be what your looking for, but this is what I need.

vacilando’s picture

vacilando commented

Excellent effort. Subscribing!

csevb10’s picture

csevb10 commented
Status: Needs review » Fixed

I went ahead and pushed forward with your solution. Regardless of the path, it seems that it creates 1 step of possible confusion/setup to get the module moving. This obviously has its strengths and weaknesses, but, for now, I'm going to side with not breaking things (I'd expect this is the 20% case) over making it easier to set up (what I would expect to be the 80% case). We'll see if I change my mind over time, but this has now been pushed into the dev branch and documentation has been added to assist in the setup.
Thanks!

Status: Fixed » Closed (fixed)

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