wpsharks / comet-cache

An advanced WordPress® caching plugin inspired by simplicity.
https://cometcache.com
GNU General Public License v3.0
75 stars 17 forks source link

Feature Request: Compatability with AdRotate #56

Open undershirtguy opened 10 years ago

undershirtguy commented 10 years ago

Feature Request: Build in the necessary hooks to allow adrotate (http://www.adrotateplugin.com) to integrate to Quick Cache.

Details: Currently the AdRotate plugin supports W3 Total Cache and WP Super Cache, whereby it can wrap Adrotate codes in cache exclusion code. This allows their ad impression and click count to be more accurate.

Currently with adrotate and quick cache installed, the AdRotate impression count shows about 1/10th of the actual impressions an ad would get on my site based on the daily number of actual pageviews i normally get.

It's not clear if that functionality (some way to call caching exclusions) is already present in your plugin, but if it is not, it would be great to have the feature added so i don't have to switch over to those other caching plugins.

The developer of AdRotate said that if there were appropriate hooks, he would build in capability for Quick Cache into his plugin.

If this functionality does exist, could you point me to the documentation so I could provide it to the AdRotate developer?

Thanks.

jaswrks commented 10 years ago

It's not clear if that functionality (some way to call caching exclusions) is already present in your plugin, but if it is not, it would be great to have the feature added so i don't have to switch over to those other caching plugins.

This functionality does not yet exist in Quick Cache I'm afraid (lite or pro).

@raamdev and I have been discussing this as a possible future addition; but given the overhead that comes with it, it's a discussion that is still being hashed out at this point.

@raamdev The question in my mind is... is there a way we can add this rarely-used feature into Quick Cache w/o it impacting (e.g. slowing down) the processing routine for site owners that are not using it?

undershirtguy commented 10 years ago

@JasWSInc just an fyi -- adrotate has been downloaded around 615,000+ times. it also has hooked into similar functionality that is offered by w3 and wp super and their product "brands" are presented to every person who installs adrotate (because they can see them in the config screen). it's kind of "free" brand exposure.

while i can't guarantee it, it is possible that if you supported this feature for adrotate and the developer of adrotate in-turn supported quick cache, he might be willing to write a blog post on his site stating support for quick cache and that would provide additional exposure to quick cache.

not sure if you care about added product exposure or not, but i thought i'd mention it just in case.

jaswrks commented 10 years ago

Ah, great tip. Thank you :-)

jaswrks commented 10 years ago

Referencing this post at WordPress.org also. http://wordpress.org/support/topic/incompatibility-with-wp-pollspls-help?replies=3#post-5121690

fox-didl commented 10 years ago

Oh yeah. This feature would be sooooo great. Can't use Quick Cache on some of my sites, because of AdRotate. There is no other option for AdRotate Plugin, so Quick Cache musst have to be deactivated.

cnxsoft commented 10 years ago

I'll also need to use Adrotate in about two weeks. I could probably have the cache's Automatic Expiration Time to 5 minutes or so, to somewhat mitigate the issue, but this would probably negate the advantage of having a cache plugin. Now W3 Total Cache is the only option for proper support of Adrotate. I've read WP Super Cache dropped support.

jaswrks commented 10 years ago

I am taking another look at this now.

jaswrks commented 10 years ago

Referencing: http://www.w3-edge.com/weblog/2013/09/w3-total-cache-pro/ Referencing: http://blog.sucuri.net/2013/05/w3-total-cache-and-wp-super-cache-vulnerability-being-targeted-in-the-wild.html

jaswrks commented 10 years ago

I took a much closer look at this. @raamdev Would you like me to implement this?

raamdev commented 10 years ago

@jaswsinc Yes, that would be great if you could tackle this for the Next Release. :) Can you assign the milestone and assign it to yourself if you'd like to take it?

jaswrks commented 10 years ago

Assigning this to me. Thanks!

jaswrks commented 10 years ago

I'm going to start work on this once we have the two outstanding PRs merged into the dev branch. I'm just going to wait until you can review that, because I'd like to start working from the latest dev branch when I implement this.

jaswrks commented 10 years ago

@raamdev I think I've knocked out all of the other issues assigned to me, except this one. Once you get the chance to review the outstanding PRs I will begin work on this one next.

raamdev commented 10 years ago

Assigning this to the Next Release milestone.

jaswrks commented 10 years ago

Referencing: http://ocaoimh.ie/2013/05/01/mfunc-in-wp-super-cache-1-4-and-beyond/ and http://wordpress.stackexchange.com/questions/98974/exclude-certain-block-from-caching-using-fragment-caching-doesnt-work

jaswrks commented 10 years ago

Couple of points I'd like to make before I begin work on this. @raamdev can make the final call about whether we should proceed with this or not :-)

My initial thought on this feature was that we should not support it. Then, after some folks in the community showed more interest it, I was willing to give it further consideration; and I planned to go ahead and try to get this into Quick Cache Pro at the very least. However, now I'm back to not liking the idea at all again. lol

My issues with this feature...


In summary, I don't like this idea at all. I can understand why some folks might like to experiment with this functionality in other caching plugins, but IMO it's better for WordPress plugins to use JavaScript snippets when they need to circumvent a page caching plugin and deliver portions of the page dynamically.

I have not tested the Ad Rotate plugin myself, but I feel sure that there are other plugins on the market that would allow for JavaScript to be used; so that content could continue to be delivered dynamically while the remaining page content remains cached as usual. This is really the best way to pull a portion of the document out of the cache itself; in my view.

<script type="text/javascript">
     $('#banner').load('http://www.example.com/wp-content/plugins/banner-plugin/load.php');
</script>

@raamdev What do you think. Is this worth doing? Or, is there a better way that you might have in mind? My feeling at the moment is that until we find a better way to accomplish this; i.e. a better method than what has been used in the recent past by W3 and Super Cache; that we should just sit on this and think about it some more.

undershirtguy commented 10 years ago

for what it's worth, if there is an alternate way to prevent ads from being cached, leveraging the built-in functionality of the adrotate plugin or quick cache, that can be accessed/managed from within admin, i'm open to trying it out.

once theme customization or special coding is required, it really becomes more complicated for us non-developers / site operators to deal with.

for practical purposes, if you don't want it in quick cache, i understand, but would be willing to work with you to test any other

alternative approaches.

best, tug undershirtguy.com http://www.undershirtguy.com (or my rss feed http://www.undershirtguy.com/feed/)

On 6/18/2014 8:57 AM, JasWSInc wrote:

Couple of points I'd like to make before I begin work on this. @raamdev https://github.com/raamdev can make the final call about whether we should proceed with this or not :-)

My initial thought on this feature was that we should not support it. Then, after some folks in the community showed more interest it, I was willing to give it further consideration; and I planned to go ahead and try to get this into Quick Cache Pro at the very least. However, now I'm back to not liking the idea at all again. lol

  My issues with this feature...

*

  There have been a few security issues related to
  dynamic/fragment caching in WordPress via |mfunc| and |mclude|.
  It looks like WP Super Cache and W3 Total Cache were both bit by
  this since they are using HTML comments in the syntax parser to
  deal with fragment caching.

  Someone leaving a comment on the site, or posting content via
  bbPress (or by other means) might be able to slide a comment
  through; where it was possible to hack into the fragment caching
  parser and inject code into the site.

  The solution that seems to have done the trick, at least in the
  short term is to require a secret code in the comment parser;
  and then strip those out before rendering the document. I really
  do NOT like this at all. It still seems very prone to security
  problems.

*

  We could support this functionality in one way or another; but
  when/if it is used, it WILL degrade performance considerably.
  There is the need to run the dynamic function or include itself;
  so that's performance hit number one. Then, in order to maximize
  compatibility in the dynamic fragments, QC will need to delay
  its delivery of these fragments until after WordPress loads;
  otherwise any dynamic function/include will not have WordPress
  available to it. That's performance hit number two, and it's a
  big one.

In summary, I don't like this idea at all. I can understand why some folks might like to experiment with this functionality in other caching plugins, but IMO it's better for WordPress plugins to use JavaScript snippets when they need to circumvent a page caching plugin and deliver portions of the page dynamically.

I have not tested the Ad Rotate plugin myself, but I feel sure that there are other plugins on the market that would allow for JavaScript to be used; so that content could continue to be delivered dynamically while the remaining page content remains cached as usual. This is really the best way to pull a portion of the document out of the cache itself; in my view.

|

|

@raamdev https://github.com/raamdev What do you think. Is this worth doing? Or, is there a better way that you might have in mind? My feeling at the moment is that until we find a better way to accomplish this; i.e. a better method than what has been used in the recent past by W3 and Super Cache; that we should just sit on this and think about it some more.

— Reply to this email directly or view it on GitHub https://github.com/websharks/quick-cache/issues/56#issuecomment-46455320.

jaswrks commented 10 years ago

once theme customization or special coding is required, it really becomes more complicated for us non-developers / site operators to deal with.

@raamdev I agree, and this is another reason why I don't care for the mfunc and mclude functionality. The only way for a site owner to actually use this, is for the plugin developer to integrate this for them. Also, since each WP caching plugin has a slightly different implementation, even the plugin developers that DO support this, might get it to work in one but not in the other. I'd really like to find a better way :-)

@undershirtguy I'll keep looking into this and update you here :-)

jaswrks commented 10 years ago

It seems to me that more often than not, this functionality is used together with certain Widgets, or with certain Shortcodes. It might be worth looking at a Quick Cache shortcode or Widget add-on that would allow a site owner to do something like this.

[quick_cache_exclude]
    [my_theme_shortcode /] or some other content here.
[/quick_cache_exclude]

Then, Quick Cache could work out a way to exclude that particular fragment on it's own; making it easier for a site owner to implement this themselves, by just using the shortcode provided by Quick Cache. We might also take it upon ourselves to try and standardize this across all of the caching plugins by also supporting a shortcode alias like [donotcache][/donotcache]; where other WP caching plugins might add support for it in the future.

In the case of a Widget there could be a checkbox to exclude the widget from being cached.


This would still degrade performance, but it would avoid a security issue and make it compatible with any theme/plugin that introduces a widget or provides a shortcode. For that matter, the shortcode could be used to wrap any sort of content, whether it be a shortcode itself; or not.

~ Just an idea. I think it needs further consideration.

undershirtguy commented 10 years ago

seems like it would work easy enough if that were possible -- not too dissimilar to how doubleclick uses the ord=[timestamp] value to force ad images to always have different img src values, that can be called from javascript or iframe code. they refer to it as cache busting, but it's only available using the doubleclick ad serving platform. i use adrotate as my adserving tool.

at the core of adroate, you define ads, and within the ads you can embed any code that will generate and render the ad. once the ad is configured, you can make it part of a group (so you can rotate more than one ad in a placement) or just display the ad alone.

adrotate provides their own widget, where you simply drag the widget to the place you want, and then define which ad or group you want to show referencing the ad or group number. all the actual code is inside the adrotate ad definition that was first created, so that's where the "exclude" code would have to go. then when the ad got rendered by the widget, it would have the exclude code around it (we'd just need to find a way to make sure that exclude parameters weren't rendered to the front end (visible to the site visitor)

for other ad placements not controlled by a widget, this would work as well because we'd just do the following: original: [adrotate banner="4"] (this renders banner 4 that is configured in adrotate new: [quick_cache_exclude][adrotate banner="4"][/quick_cache_exclude]

hope this info provides some context with adrotate.

best, tug undershirtguy.com http://www.undershirtguy.com (or my rss feed http://www.undershirtguy.com/feed/)

On 6/18/2014 9:34 AM, JasWSInc wrote:

It seems to me that more often than not, this functionality is used together with certain Widgets, or with certain Shortcodes. It might be worth looking at a Quick Cache shortcode or Widget add-on that would allow a site owner to do something like this.

[quick_cache_exclude] [my_theme_shortcode /] or some other content here. [/quick_cache_exclude]

Then, Quick Cache could work out a way to exclude that particular fragment on it's own; making it easier for a site owner to implement this themselves, by just using the shortcode provided by Quick Cache. We might also take it upon ourselves to try and standardize this across all of the caching plugins by also supporting a shortcode alias like |[donotcache][/donotcache]|; where other WP caching plugins might add support for it in the future.

In the case of a Widget there could be a checkbox to exclude the widget from being cached.


This would still degrade performance, but it would avoid a security issue and make it compatible with any theme/plugin that introduces a widget or provides a shortcode. For that matter, the shortcode could be used to wrap any sort of content, whether it be a shortcode itself; or not.

~ Just an idea. I think it needs further consideration.

— Reply to this email directly or view it on GitHub https://github.com/websharks/quick-cache/issues/56#issuecomment-46460115.

raamdev commented 10 years ago

There have been a few security issues related to dynamic/fragment caching in WordPress via mfunc and mclude. It looks like WP Super Cache and W3 Total Cache were both bit by this since they are using HTML comments in the syntax parser to deal with fragment caching.

Yeah, I researched that topic a bit too and saw what a mess it was for those plugins when that vulnerability was found. I'm far more concerned about keeping Quick Cache secure than with being compatible with all plugins.

That said, excluding specific dynamic portions of a page from the cache has been a much-requested feature--this specific feature request for AdRotate compability is really part of the bigger feature request of being able to keep certain parts of a cached page dynamic.

We could support this functionality in one way or another; but when/if it is used, it WILL degrade performance considerably. There is the need to run the dynamic function or include itself; so that's performance hit number one. Then, in order to maximize compatibility in the dynamic fragments, QC will need to delay its delivery of these fragments until after WordPress loads; otherwise any dynamic function/include will not have WordPress available to it. That's performance hit number two, and it's a big one.

I agree. No matter how you look at it, supporting dynamic fragments will result in big hits to performance, which Quick Cache is supposed to be all about improving. On the surface, it seems obvious that it doesn't make sense to go down a path that will clearly degrade performance. Yes, this could be an "optional" feature, but we also need to take into account the security implications mentioned above.


Given the above, I would like to explore other options for dynamic fragmentation. Until we come up with a solution, there's not much we can do to provide additional support for the AdRotate plugin.

For now, I'm opening a new issue for this topic and will explore this further in a future release cycle and at this time I will recommend that site owners contact plugin developers and ask them for a way to include the dynamic portions of the plugin using Javascript so they can be compatible with WordPress caching plugins. Something like @jaswsinc's example above:

<script type="text/javascript">
     $('#banner').load('http://www.example.com/wp-content/plugins/banner-plugin/load.php');
</script>

I'll leave this issue open for now, until we come to a resolution with dynamic fragmentation.

raamdev commented 10 years ago

@undershirtguy writes...

hope this info provides some context with adrotate.

Thank you very much! That provides a lot of very helpful context and information. It sounds like using a [quick_cache_exclude][/quick_cache_exclude] shortcode may be the best way to go with this.


I'm going to track further discussion on the larger topic of Dynamic Fragmentation in #222.

If you have any additional information to add regarding AdRotate, please feel free to add that here. :)

jaswrks commented 10 years ago

hope this info provides some context with adrotate.

@undershirtguy Yes, that's great! Thank you.