I'm using Drupal 7.31 with version 2.2 of the twitter block. If the JS aggregation performance setting is enabled, then no twitter feeds are displayed. If the setting is disabled, the twitter block works fine.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

johan.gant’s picture

Priority: Normal » Major

Yep, I can confirm the same happens when upgrading from 2.1 to 2.2.

robinsonsarah01’s picture

I've been having the same issue with 2.2, and making sure that preprocessing of the file is off seems to fix the issue (adding a "'preprocess' => FALSE," option when attaching the javascript). The problem doesn't crop up with the external file because external js files are never aggregated.

robinsonsarah01’s picture

Status: Active » Needs review
FileSize
326 bytes

Attaching a patch for this. As what might be a temporary solution, preventing aggregation works so far.

bneil’s picture

I was having the same problem, but noticed that #2266961: loading of the twitter js widget causes the site to slow down creates a local copy of the javascript file on cron. Can you try running cron and see if it starts to work?

GoofyX’s picture

@bneil, no it doesn't seem to work. The moment you enable JS aggregation, the block does not appear. Disabling it and voilá!

webdrips’s picture

Can confirm patch fixed problem

mysty’s picture

patch fixed non updating tweets in tweet block on a site with Omega8 hosting.

Their platform uses a lot of automatic aggregation, unless explicitly forced off via https://omega8.cc/how-to-disable-all-caching-and-aggregation-115

kyletaylored’s picture

Status: Needs review » Reviewed & tested by the community

This definitely worked for us. What an odd bug. Setting to RTBC.

serg2’s picture

Issue summary: View changes
serg2’s picture

Issue summary: View changes
d.sibaud’s picture

it works fine

jimmynash’s picture

Just chiming in. This patch worked for me as well.

serg2’s picture

This patch is more of a work-around than a solution so I would suggest that it needs work rather than be committed.

The patch prevents aggregation as described by the patch author at #3 . Roughly speaking the point of aggregation is that it speeds up websites. To simply disable it for the Twitter script is not fixing the issue, but ignoring it.

Another workaround to this issue is using the AdvAgg module: Advanced CSS/JS Aggregation (https://www.drupal.org/project/advagg) . This brings other benefits as well, alongside very good troubleshooting and loads of tweaking options.

I found that the Twitter block fails if minified with JSMIN+ rather than JSMIN on some systems.

hitfactory’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.13 KB

I think the problem here is the absolute file path returned by twitter_block_cache().

Attached patch returns a relative path e.g. sites/default/files/twitter_block/widget.js instead of the current http://example.com/sites/default/files/twitter_block/widget.js

Now you can enable JS aggregation without a hitch.

joshuautley’s picture

#14 does not work on Pantheon

badbanana’s picture

+1 to #14. Our issue was that we have multiple domains for our site, and twitter_block wasn't rendering under all of them. @hitfactory's patch cleared it right up.

hitfactory’s picture

@joshuautley, have tested on a Drupal 7 install on Pantheon with just this module also enabled. What errors are you seeing?

joshuautley’s picture

@hitfactory - Pantheon DEV and TEST experienced a 502 error within the Pantheon environment.

hitfactory’s picture

@joshuautley, the twitter_block_cache() function only runs on cron or when you view the actual block.

What did you do in the Pantheon UI before the 502? Press Clear Caches?

joshuautley’s picture

Yes. When cloning the db and file from LIVE to TEST Pantheon gives you the option to clear cache's which I always do.

johan.gant’s picture

I think the next step on this issue is for the maintainer (ZenDoodles) to consider merging the patch into the dev branch and/or considering a new release tag. I think the Pantheon configuration options are a tangent and would be better suited to another place or issue... Pantheon developer support, perhaps?

joshuautley’s picture

Clearing cache in Pantheon is not a "configuration option" it is just an option and is the same as clearing the cache in Drupal itself.

Now I do understand there may be some incompatibility issue with Pantheon system but they didn't seem to have a clue as to why their system was throwing a 502 error. I ultimately figured it out.

I will post a link to this discussion to my (now closed) Pantheon support ticket to see if they feel like looking into it on their end.

joshk’s picture

@joshuautley - if you know the issue we'd be glad to hear it. A 502 is generally a segmentation fault, so not something that's easily debugged. It's not uncommon for code that works in a super-vanilla PHP environment to have issues when run with opcode caching/etc enabled. There are definitely cases with object structure or recursion that can cause a 502.

joshuautley’s picture

@joshk - The following is the patch I applied that caused the 502 error.

diff --git a/twitter_block.module b/twitter_block.module
index bca3a56..e5e7152 100644
--- a/twitter_block.module
+++ b/twitter_block.module
@@ -488,6 +488,7 @@ function twitter_block_cache($sync_cached_file = FALSE) {
   $location = 'http://platform.twitter.com/widgets.js';
   $path = 'public://twitter_block';
   $file_destination = $path . '/' . basename($location);
+  $wrapper = file_stream_wrapper_get_instance_by_uri($file_destination);
 
   if (!file_exists($file_destination) || $sync_cached_file) {
     // Download the latest widget code.
@@ -517,14 +518,14 @@ function twitter_block_cache($sync_cached_file = FALSE) {
           watchdog('twitter_block', 'Locally cached widget code file has been saved.', array(), WATCHDOG_INFO);
 
           // Return the local JS file path.
-          return file_create_url($file_destination);
+          return $wrapper->getDirectoryPath() . '/' . file_uri_target($file_destination);
         }
       }
     }
   }
   else {
     // Return the local JS file path.
-    return file_create_url($file_destination);
+    return $wrapper->getDirectoryPath() . '/' . file_uri_target($file_destination);
   }
 }
 

Do you see how/if any of these changes could cause an issue within the Pantheon system?

robinsonsarah01’s picture

This is fixed for me in 2.3. Interestingly, it's also no longer an issue in 2.2, so some change to my environment in the last year seems to have made this no longer a problem for me.

blasthaus’s picture

Keep in mind the only way to test this code is if the file either does not exist or the http request to twitter somehow fails.

file_stream_wrapper_get_instance_by_uri() will return FALSE if no registered handler could be found

So this has to be checked first before it can work in all use cases. This is likely the case for Pantheon @joshuautley

I propose to simply alter the return only if file_stream_wrapper_get_instance_by_uri can find a wrapper.

Patch attached.

blasthaus’s picture

It would be a good idea to also check that the wrapper is a local one too, so here's an updated patch.

naveenvalecha’s picture

Version: 7.x-2.2 » 7.x-2.x-dev
Status: Needs review » Needs work
Issue tags: +Novice, +Needs tests

Thanks for the patch,
Have you tested it against 7.x-2.x

smustgrave’s picture

Status: Needs work » Closed (outdated)

Closing as outdated as D10 just released so focus will be going toward D9/D10 support mainly.

Will keep an eye on the 7.x branch for reviews.