Addthis loads multiple identical links in the header to the CSS files from addthis.com and completely wipes out the all of the other styles in all versions of IE when it appears on the homepage (looks like none of the Drupal core CSS or the theme specific CSS gets loaded by IE at all). It basically renders the module completely unusable since so many people still refuse to get a better browser. We had to disable it on the site we experienced it on so I cannot show a you a (non)working example.

CommentFileSizeAuthor
#13 addthis.module.PATCH953 bytesfabianderijk

Comments

bacon’s picture

Yes. I just noticed the same behavior in IE 8.0.6001.

There are multiple instances (6-8) of the following line at the top of the web page source before the <title>.
<link rel="stylesheet" type="text/css" href="http://s7.addthis.com/static/r07/widget02.css" />

The following code in addthis.module may be the culprit.

  // Fix IE's bug.
  if (strpos($_SERVER['HTTP_USER_AGENT'], 'MSIE') !== FALSE) {
    drupal_add_link(array(
      'rel' => 'stylesheet', 
      'type' => 'text/css',
      'href' => "http://s7.addthis.com/static/r07/widget02.css",
    ));
  }

I know I shouldn't have, but I went ahead and commented out the above code, downloaded the widget02.css file, put it in the addthis folder and edited the addthis.module to do a drupal_add_css on the widget02.css.

Firetracker’s picture

Hi,

I can confirm that this is happening too in IE6. Only happens on a page that loads in a GMAP. I've rolled back to 6.x-2.7 to fix the issue.

Cheers
Zap

rapsli’s picture

me too. This server side browser distinction doesn't make any sense either! Let's assume we are using a cache Boost or probably even the standard Drupal cache. The complete rendered HTML is being stored, well depending on what browser builds the cache we have the css loaded or not. Using conditional CSS would be a better solution for this!

vesapalmu’s picture

Status: Active » Needs work

I agree that this should be changed to conditional CSS. Any patches that are tested to work with IE are welcome.

dgale’s picture

Just to let everyone know, this problem isn't specifically caused by AddThis. The real problem is that IE has a harcoded limit of 31 stylesheets. If you are close to this limit and using 6.x-2.8 of AddThis, those extra 6-8 references just might push you over the edge.

There is still obviously the problem of AddThis adding those extra duplicated references. But if your site has a lot of modules, you might hit the limit anyway.

To get around the problem, you can try turning on Optimize CSS files at /admin/settings/performance. If you are unlucky, this will cause your webserver to choke. Read here http://drupal.org/node/444228 for a detailed discussion of *that* problem (and thankfully a few patches to try.)

While researching this problem, I also came across this module http://drupal.org/project/unlimited_css which seems like it also might address the IE problem. I haven't tried it myself, but there you go.

Sort of back to the original question . . . is there a reason the stylesheet widget02.css for this module points to an external site as opposed to being part of the module itself?

vesapalmu’s picture

Priority: Critical » Normal

The reason for pointing CSS to external site is two fold: 1) Originally addthis.com forced you to use their version of JS+CSS. 2) Addthis.com uses CDN which is faster than most Drupal servers and also way more likely to be cached on visitors browser. That said we will add an option to cache locally everything from Addthis.com in 3.0 version.

I will lower the priority of this issue since it really only has critical effects in a very specific setup. The bug itself needs to be fixed anyway.

ChrisRut’s picture

Title: 6.x-2.8 wipes out all styles in IE, multiple addthis css files loaded » wipes out all styles in IE, multiple addthis css files loaded
Version: 6.x-2.8 » 6.x-2.9

Still having issues with this in 6.x-2.9 even with Optimize CSS enabled (admin/settings/performance)
Also tried Unlimited CSS module, no go with Optimize CSS on or off
Tested in IE6,7,8

Solution in #1 solved it, Thanks bacon.

AlexisWilke’s picture

There is another side effect to that add: I use AddThis on a secure connection and adding a link to an unsecure server generates warnings in IE. That's a problem since such errors tend to make people think that they should not proceed (and when that happens in your cart, you lose sales!)

So, may we at least change that link to an HTTPS version? It shouldn't cause any problem when not secure. We could also test whether we should use HTTP or HTTPS... (use $_SERVER['HTTPS'] for the condition).

Also, I think that the original problem may come from people who have more than one AddThis on a page. Then the function may get called more than once and thus the link added multiple times. The following includes a potential fix for that problem. Let us know if that fix your duplicates. I do not see any duplicates on my end, but I do have just one AddThis button.

  // Fix IE's bug.
  static $fix_ie_bug = TRUE;
  if ($fix_ie_bug && strpos($_SERVER['HTTP_USER_AGENT'], 'MSIE') !== FALSE) {
    $fix_ie_bug = FALSE;
    drupal_add_link(array(
      'rel' => 'stylesheet',
      'type' => 'text/css',
      'href' => "https://s7.addthis.com/static/r07/widget02.css",
    ));
  }

The following would be checking whether we have a secure server or not:

      'href' => ($_SERVER['HTTPS'] == 'on' ? "https://secure" : "http://s7") . ".addthis.com/static/r07/widget02.css",

Thank you.
Alexis

P.S. I changed the secure entry since it looks like you may need/want to use https://secure.addthis.com/ ... It worked with s7 too though.

AlexisWilke’s picture

I posted a patch for that HTTPS problem and other HTTPS problems in #736816: PHP notice Undefined index: HTTPS

Thank you.
Alexis Wilke

mstrelan’s picture

Status: Needs review » Needs work

#8 is the way to go. If you wanted to use conditional comments you could use drupal_set_html_head()... for example

<?php
  static $fix_ie_bug = TRUE;
  if ($fix_ie_bug) {
    $fix_ie_bug = FALSE;
    $https = isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on';
    $ie_css_path = ($https ? "https://secure" : "http://s7") . ".addthis.com/static/r07/widget02.css";
    drupal_set_html_head('<!--[if IE]><link rel="stylesheet" type="text/css" href="' . $ie_css_path . '" /><![endif]-->');
  }
?>
mstrelan’s picture

Status: Needs work » Needs review
haopei’s picture

Status: Needs work » Needs review

Hello, I am not familiar with PHP. What do I do with this?

fabianderijk’s picture

StatusFileSize
new953 bytes

Hi,

I've created a patch for this issue. It's against the 6.x.2.10 version of this module, but it solves it all (in my case).

xtfer’s picture

Status: Needs review » Needs work

#10 isn't a fix for review, its a suggestion for a fix.

#13 appears to be a fix for #654180: php notices with some overlap on this issue.

This issue still needs a fix.

AlexisWilke’s picture

Yeah... Interestingly enough I posted a patch in #736816: PHP notice Undefined index: HTTPS in comment #5 and then Dave simply marked the post as a duplicate. Bye bye patch... 8-P

Thank you.
Alexis Wilke

eduserrat’s picture

Thanks "AlexisWilke", it worked for us.
We put your code in "addthis.module":

<?php
  // Fix IE's bug.
  static $fix_ie_bug = TRUE;
  if ($fix_ie_bug && strpos($_SERVER['HTTP_USER_AGENT'], 'MSIE') !== FALSE) {
    $fix_ie_bug = FALSE;
    drupal_add_link(array(
      'rel' => 'stylesheet',
      'type' => 'text/css',
      'href' => "https://s7.addthis.com/static/r07/widget02.css",
    ));
  }
?>

The only thing we saw: "https://s7.addthis.com/static/r07/widget02.css" was old, and we replaced it for "http://s7.addthis.com/static/r07/widget66.css" (the new one).

Thanks !

gisle’s picture

Issue summary: View changes
Status: Needs work » Closed (outdated)

This fix is for an outdated version of the module and for an outdated version of MSIE.

Feel free to repoen if you still think this need to be fixed for the 7.x-4.x branch and modern browsers.