I get mixed results on different pages with Views Carousel. Sometimes the skin styles are picked up by IE and sometimes they are not. I cannot determine what the differences are between pages except perhaps other javascript files associated with other modules. However, the same pages consistently do not include the skin styles.

Wondering if this is because the reference to skin.css it is currently inserted so late in the list of style sheets?

CommentFileSizeAuthor
#10 jcarousel-648254-v2.patch700 bytesAdrianB
#8 jcarousel-648254.patch463 bytesAdrianB

Comments

eft’s picture

eft commented
Project: Views carousel » jCarousel
Version: 6.x-2.x-dev » 6.x-1.x-dev

In the function jcarousel_add in jcarousel.module the skin style is added with the $type argument set to 'theme'. This has the effect of referencing the skin styles later than if it were set to 'module'. Would this be why IE is not referencing the skin styles properly?

....
// Add the skin CSS if it hasn't been added yet.
    if (!isset($jcarousel_added['skin'][$skin])) {
      if ($skin == 'tango' || $skin == 'ie7') {
        drupal_add_css("$jcarousel/jcarousel/skins/$skin/skin.css", 'theme');
      } 
      elseif (!empty($skin_path)) {
        drupal_add_css($skin_path, 'theme');
      }
      $jcarousel_added['skin'][$skin] = TRUE;
eft’s picture

eft commented

I found out the cause of my problem. IE does not recognize more than 30 style tags. Turning on CSS aggregation (/admin/settings/performance --> Turn on CSS Optimization) is the solution.

Hope this helps someone out there.

robloach’s picture

robloach
he/him
commented
Status: Active » Fixed

Yay IE! Maybe this will go away next year:
http://geeksmack.net/component/content/article/867-microsoft-ending-life...

Status: Fixed » Closed (fixed)

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

maddentim’s picture

maddentim commented
Status: Closed (fixed) » Active

Hi, this is exclusive to IE. I am having issues with overriding the styling in FF... I see my customizations in Firebug, but they are superceded by the module's css file. Or maybe I am doing something else wrong. Maybe I need to just make my own theme.

AdrianB’s picture

AdrianB commented

This must be a bug.

As eft wrote in #1, starting on line 136 in jcarousel.module there's this code:

    // Add the skin CSS if it hasn't been added yet.
    if (!isset($jcarousel_added['skin'][$skin])) {
      if ($skin == 'tango' || $skin == 'ie7') {
        drupal_add_css("$jcarousel/jcarousel/skins/$skin/skin.css", 'theme');
      } 
      elseif (!empty($skin_path)) {
        drupal_add_css($skin_path, 'theme');
      }
      $jcarousel_added['skin'][$skin] = TRUE;
    }

Why would a module use theme instead of module?

This caused skin.css to be loaded after theme css files and therefore you cannot override the skin.css with your own css in your theme.

Changing theme to module fixes this.

This has nothing to do with IE afaik.

Please change this and release a new official release (as that would fix #655142: 6.x-2.x-dev Circular wrapping doesn't work ( for me ! ) as well).

AdrianB’s picture

AdrianB commented
Title: IE doesn't pick up styles in skin.css » Use "module" instead of "theme" when adding skin.css

I took the liberty to change to issue title.

AdrianB’s picture

AdrianB commented
StatusFileSize
new jcarousel-648254.patch463 bytes

It just a change on one word, but here's a patch non the less. Please test.

(Be gentle, this is my first patch ever, hopefully I did it the right way :)).

AdrianB’s picture

AdrianB commented
Status: Active » Needs review
AdrianB’s picture

AdrianB commented
StatusFileSize
new jcarousel-648254-v2.patch700 bytes

Actually, there's two places to change in 6.x-1.x-dev, here's a new patch.

AdrianB’s picture

AdrianB commented

I think this patch actually helps, so I would very much like some kind of response from the maintainers.

quicksketch’s picture

quicksketch
he/him
commented
Status: Needs review » Fixed

Hi Adrian, sorry I didn't spend time looking at this patch, I've been working on merging Views Carousel and jCarousel together, which is now completed in the new 2.x branch. This problem does not exist there, I'd encourage you to try the new version (which will probably require some changes on your part), which shouldn't have this problem.

AdrianB’s picture

AdrianB commented

Ah, I will do that. But just out of curiosity: Am I right that modules should use drupal_add_css($skin_path, 'module'); rather than drupal_add_css($skin_path, 'theme');?

quicksketch’s picture

quicksketch
he/him
commented

Yes that's generally right. Some modules will add as "theme" to make it so that their CSS files come later than other CSS files, otherwise the order of the CSS files is determined entirely by the order that drupal_add_css() is called.

Status: Fixed » Closed (fixed)

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