Don't restrict the default layout, fonts and custom CSS files only to the screen media type [#2089541]

Currently, even if responsive layout and styles are disabled in the CSS Global Settings of an AdaptiveTheme sub-theme, the default layout, fonts and custom CSS files are always only loaded to the screen device. The resulting HTML code loading these CSS files will be something like this (here using the AT Commerce sub-theme):

<style media="screen">@import url("http://www.example.com/sites/example.com/files/adaptivetheme/at_commerce_files/at_commerce.default.layout.css?mt600m");
@import url("http://www.example.com/sites/example.com/files/adaptivetheme/at_commerce_files/at_commerce.fonts.css?mt600m");
@import url("http://www.example.com/sites/example.com/files/adaptivetheme/at_commerce_files/at_commerce.custom.css?mt600m");</style>

Note the the "media" attribute is explicitly set to "screen". As a consequence, a printer or any other non-screen device never gets this CSS content. Thus, the print out of any page using a AdaptiveTheme sub-theme really looks ugly.

Comment	File	Size	Author
#1	adaptivetheme_2089541.patch	1.17 KB	roball

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

roball CreditAttribution: roball commented 15 September 2013 at 17:08

Status:

Active

» Needs review

File	Size
adaptivetheme_2089541.patch	1.17 KB

The attached patch fixes this issue. It effectively removes the media="screen" attribute, so the CSS gets loaded to all devices, most notably to the media type "print".

Comment #2

Jeff Burnz CreditAttribution: Jeff Burnz commented 15 September 2013 at 22:53

Category:

bug

» feature

I'm not entirely convinced this is a bug, AT ships with a print.css file that is supposed to be used for this sort of thing, it hides a lot of stuff but it does predate some of the 7.x-3.x changes such as this sort of blanket "media screen" setting, so if a there is a bug its a lack of flexibility to do both - ideally one can prevent screen styles bleeding into print as this is often unwanted, and that is what AT does, but fails to allow it to happen should you want it to.

I think about it for a bit, however we can't do that patch imo, it would potentially break a lot of sites who have followed a different approach (those that do not expect screen styes in their print). If anything we'd need a way to switch the media type in certain situations, which should be doable in one way or another.

I recall in the past I have suggested using @import to get what you want into the print.css, not an ideal solution but it does work at a pinch. But yeah, being able to choose is the best solution, so needs some thought as to how that can work.

Comment #3

roball CreditAttribution: roball commented 17 September 2013 at 12:06

Title:

Load default layout, fonts and custom CSS files not only to screen media

» Load default layout, fonts and custom CSS files not only to screen media type

OK, let's treat this as a feature request. The only print.css AT ships with is one in at_subtheme/css/. I am using the AT Commerce sub-theme, so print.css never gets loaded. Anyway, being dependent on a separate, dedicated CSS file that only contains the print media styles is a bad, old-school approach because you would have to duplicate a lot of CSS code into that file.

To get the most freedom, the best way is to simply @import all CSS files within a plain
<style></style> tag, without limiting it to a certain media type per se such as media="screen". If you want to limit the specified styles to the screen media type, you can still do so within the CSS files itself, by wrapping all content inside the CSS files within

@media screen {

  /* all definitions here */

}

For the custom CSS file (*.custom.css), customers should be allowed to decide themselves which media types code should apply to, by using

@media screen, print {

  /* definitions for both screen and print media types here */

}

in some parts, and maybe only screen in other parts. This however requires the hardcoded media="screen" attribute to be removed, as suggested in my patch.

Comment #4

Jeff Burnz CreditAttribution: Jeff Burnz commented 17 September 2013 at 22:28

Priority:	Major	» Normal
Status:	Needs review	» Needs work

I'm thinking more along the lines of a setting that allows you to switch the media type to ALL if you want to, and leaving the current default for the life of the major version (7.x-3.x). I'm pretty hesitant to just remove it, but thats not saying I won't - I want to test out a few scenarios first and see what haps, honestly when I look at the code again I reckon it could be removed, but a setting might work as well and remove all doubt over BC.

Comment #5

roball CreditAttribution: roball commented 18 September 2013 at 13:24

Title:	Load default layout, fonts and custom CSS files not only to screen media type	» Don't restrict the default layout, fonts and custom CSS files only to the screen media type
Assigned:	roball	» Unassigned

Yes, implementing a setting that allows overriding the value of the media attribute from its default screen to whatever is preferred (such as screen, print or all) is also a way to solve this problem. Most important that there will be any way that drops hard-coded restriction to screen.

This will be good for the the default layout and fonts CSS, since the user is not supposed to hack into these theme generated files.

However, I don't think it's necessary for the custom CSS file, since it does not ship with the theme and it's intended to be completely controlled by the user. For that file I think it's best to simply remove 'media' => 'screen' from drupal_add_css().

Comment #6

roball CreditAttribution: roball commented 24 March 2014 at 14:40

Version:	7.x-3.x-dev	» 7.x-3.2
Issue summary:	View changes

[root@server themes]# cd adaptivetheme
[root@server adaptivetheme]# wget https://drupal.org/files/adaptivetheme_2089541.patch
[root@server adaptivetheme]# patch -p1 < adaptivetheme_2089541.patch
patching file at_core/inc/load.inc