Problem

Currently an active stylesheet loads by JS on HTML DOM ready event, i.e. a user does not see a results of their selected style until the whole page is fully rendered.
A problem of the technique used is that it is a browser based hack and it does not allow to set one of styles to be visible initially.

Proposed resolution

Instead of list of <link rel="alternate stylesheet"> tags we can use only one <link rel="stylesheet"> with 'href' attribute initially pointing to active CSS and make JS change that 'href' on switching.

Remaining tasks

Patch to come shortly.

CommentFileSizeAuthor
#5 styleswitcher-changed-switching-technique-1966982-1.patch12.64 KBrafal.cygnarowski
#1 styleswitcher-changed-switching-technique-1966982-0.patch12.09 KBpingwin4eg

Comments

pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
commented
Status: Active » Needs review
StatusFileSize
new styleswitcher-changed-switching-technique-1966982-0.patch12.09 KB

Besides the main task this patch also does following subtasks:

  • parses carefully 'styleswitcher' directive in theme.info (so no more troubles with setting/ignoring default style + uses blank CSS file if default style is not set in theme.info but there's must be an option to turn off all alternates);
  • uses 'machine' names for styles instead of titles (internally in code, titles in theme.info remains untouched);
  • uses lowercase 'styleswitcher' cookie name with style's 'machine' name as value;
  • moves init functionality to preprocess_html() (so block with list of alts does not have to be on every page);
  • moves .js file addition from .info file to preprocess_html() where all styleswitcher settings are adding;
  • adds an 'id' attribute to <link> to easily find it from JS (no 'title' attr which caused browser bugs).
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Tried using patch via git, came up with these errors (did a git clone of project)

Matts-MacBook-Pro:styleswitcher mglaman$ patch < styleswitcher-changed-switching-technique-1966982-0.patch 
patching file styleswitcher.css
patching file styleswitcher.info
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file styleswitcher.info.rej
patching file styleswitcher.js
Hunk #1 FAILED at 9.
Hunk #2 FAILED at 33.
Hunk #3 FAILED at 47.
3 out of 3 hunks FAILED -- saving rejects to file styleswitcher.js.rej
patching file styleswitcher.module
Hunk #1 FAILED at 21.
Hunk #2 FAILED at 41.
Hunk #3 succeeded at 99 with fuzz 2 (offset -15 lines).
2 out of 3 hunks FAILED -- saving rejects to file styleswitcher.module.rej
pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
commented

It seems that you are trying to apply the patch to wrong branch ('master', I think). This patch is for '7.x-1.x' branch (as stated in 'version' field of an issue).
Please checkout to 7.x-1.x and try again.

rafal.cygnarowski’s picture

rafal.cygnarowski commented

The patch have problem with different css file include techniques. In my installation default way to include css is

@imort()

when patch assumes it's
tag. Even more: if I refresh the page, then

tag still exists but with media attribute set to "styleswitcher" and tag is created with "styleswitcher-css" id. I didn't checked what is causing all the mess but it's need a change. For now following change made it work for me: 
        switchStyle: function(name) {
          var choice = Drupal.settings.styleSwitcher.choices[name];
          var ss = $('#styleswitcher-css');
          if (ss.is('style')) {
            // initially css file is provided by <style> tag
            ss.html('@import url("' + Drupal.settings.basePath + choice.path + '");');
          } else { // is link
            // but after refreshing, the <link> tag is created with styleswitcher-css id
            ss.attr('href', Drupal.settings.basePath + choice.path);

            // (old <style> tag remains and need to be removed)
            $('style[media=styleswitcher]').remove();
          }

          $.cookie('styleswitcher', name, {
            path: Drupal.settings.basePath,
            // The cookie should "never" expire.
            expires: 36500
          }
          );

        },
rafal.cygnarowski’s picture

rafal.cygnarowski commented
Issue summary: View changes
StatusFileSize
new styleswitcher-changed-switching-technique-1966982-1.patch12.64 KB

After few tests I had to made changes to deal with browser cache and make it work with IE8.

kingfisher64’s picture

kingfisher64 commented

#5 appears to work fine.

kingfisher64’s picture

kingfisher64 commented
Priority: Normal » Major

#5 is the starting point at present to get this module up and running. Apply this prior to other patches.

  • Commit b74d6d9 on 7.x-2.x by pingwin4eg: 
    Issue #1966982 by pingwin4eg: Changed style switching technique.
pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
commented

To rafal.cygnarowski:

I am trying to understand how did you run into such problem. The code you provided in patch is unneeded in normal flow:

var ss = $('#styleswitcher-css');
          if (ss.is('style')) ...

because we set 'preprocess' option to FALSE in drupal_add_css(). It means our css should always be rendered as separate <link> tag and never as <style>@import url();</style>.

So it seems like something's gone wrong in process of update from the old version to the patched one. Can you please provide more info on what did you do. Did a patch apply flawlessly? Did you clear caches after patching? What did you mean by "your installation" in phrase

In my installation default way to include css is @imort()

? What exactly could cause using @import where it should not be? What Drupal version did you use? Any other information that to your notice can be useful.

P.S.: I'll hide your patch from display in the issue because of the reason I mentioned above until more information comes.

kingfisher64’s picture

kingfisher64 commented

@pingwin4eg would you recommend that any new people download the 2.x branch from git for now?

Many thanks

pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
commented
Status: Needs review » Fixed

I've added new dev release for 2.x on project page. So yes, use it instead of 1.x.
I'm closing this issue since the patch already committed. Please start new issue for 2.x if needed.

rafal.cygnarowski’s picture

rafal.cygnarowski commented

To pingwin4eg:

It was some time ago, so I don't remember well the way to reproduce this behavior.
But, AFAIR: check how the page looks like when you enter it for the first time, and then after page refresh. It's possible that using browser private mode is required.

It was tested with Drupal 7.24 (or near this version).

rafal.cygnarowski’s picture

rafal.cygnarowski commented

Ok, I checked newest 2.x-devel and it looks as follows:

1. Newly opened browser in privet mode. The default stylesheet is loaded by following code:

<style type="text/css" media="styleswitcher">@import url("http://test.site.pl/sites/all/themes/mytheme/style-normal.css?n4j1kd");</style>

2. Switching to ohter style and next css file is loaded with following code:

<link type="text/css" rel="stylesheet" href="http://test.site.pl/sites/all/themes/mytheme/style-yellow-n-black.css?n4j1kd" media="all" id="styleswitcher-css">

Switching back to default theme is not possible because JS is generating exception:

Uncaught TypeError: Cannot read property 'path' of undefined styleswitcher.js?n4j1kd:15
Drupal.styleSwitcher.switchStyle styleswitcher.js?n4j1kd:15
(anonymous function) styleswitcher.js?n4j1kd:75
e.complete jquery.js?v=1.4.4:152
c.fx.step jquery.js?v=1.4.4:156
e jquery.js?v=1.4.4:153
c.extend.tick jquery.js?v=1.4.4:157
pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
commented

As I understand you are testing it as anonymous Drupal user, right? Can you report the status of page caching and CSS aggregation/compression in your environment?
Probably we should start another issue.

rafal.cygnarowski’s picture

rafal.cygnarowski commented

Yes, anonymous user. Anonymous pages are not cached and css files are not aggregated/compressed.

pingwin4eg’s picture

pingwin4eg
Location Zaporizhia 🇺🇦
commented
Status: Fixed » Closed (fixed)

I'm sorry but I can't reproduce that. Closing this issue. Feel free to start a separate one, but please give us a more information.