Add missing support for RTL languages
tombigel - November 23, 2008 - 07:29
| Project: | Conditional Stylesheets |
| Version: | 6.x-1.1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
I maintain a theme called Tendu, and we are just now working on a function that does something similar.
Your implementation is far better then our current approach.
My question - Can this module / technique be implemented only with a function in template.php (+ the lines in the info file)?
I would love to add it as a part to my theme, but I don't want the theme to include a modules/ directory by default.
btw - how does your module handles sub-themes? any special behavior, or does it work just like regular added css file?
Thanks
Tom

#1
With sub-themes, the same rules apply to conditional stylesheets as to regular stylesheets. Sub-themes inherit the conditional stylesheets of their parent themes; and sub-themes can override or remove conditional stylesheets in the normal way.
And, yes, you can add this technique using a tendu_preprocess_page() and a tendu_theme() function. But its tricky. Zen 6.x-1.0-beta3 (and later) does this.
#2
Thanks, I copied the relevant code from Zen and it works :)
One more question I forgot to ask:
In Drupal 6, when an RTL language is selected, for every stylesheet loaded Drupal looks for an additional stylesheet-rtl.css and adds it for incremental RTL changes right after it loads the regular stylesheet.
This rule does not apply to the stylesheets added by this module, I'll try to add it manually and post it here, but it should be a part of the module.
#3
Um… not sure. Can't check right this sec, so I'll re-open this issue so I don't forget to check later.
#4
Hi again, I created a patch for automatically adding RTL CSS files when needed.
It was tested only on my local Tendu installation, I hope it works for you too.
I hope the patch is in the right format, never created one for a Drupal project myself.
#5
Thanks for the patch! I'll take a look at it soon.
#6
btw - The code with the patch is already integrated in Tendu version 2.x-dev and 2.x beta1
#7
Found a bug in my code:
If "locale" module is not enabled, the constants LANGUAGE_RTL / LANGUAGE_LTR are not defined.
I tried to replace this line in my patch:
if ($language->direction == LANGUAGE_RTL){With this line:
if (defined(LANGUAGE_RTL) && $language->direction == LANGUAGE_RTL){But it doesn't work.
I need help with this one...
#8
Sorry my bad - it was supposed to be:
defined('LANGUAGE_RTL')
So this line:
if (defined('LANGUAGE_RTL') && $language->direction == LANGUAGE_RTL){Works!
#9
You can't add a file when editing a comment here, so here's the updated patch...
#10
Rather than using all those backslashes to escape the string delimiter inside it, I would use a different string delimiter.
#11
I changed a couple of string delimiters which should not have been changed.
#12
ok.
I just copied their original "output .=" line...
#13
So we have a choice of lots of
\"or lots of' . 'in that line. Neither are ideal, but I still find the \" easier to read.Thanks for the patch, Tom! Can I get someone to confirm the patch in #9 actually works? :-)
#14
This is the same as Tom's patch in #9 but conforms to Drupal's coding standards (with regards to spaces). Also, given how drupal_add_css() works, I've removed the check for
defined('LANGUAGE_RTL')as it seems unnecessary.#15
Tom, I just re-read your comment in #7 above, LANGUAGE_RTL and LANGUAGE_LTR are not defined in locale.module, they are defined during the bootstrap. http://api.drupal.org/api/constant/LANGUAGE_RTL/6
So, I don't understand what errors you were encountering.
#16
Ok, I created a fake language for en-us that is RTL. Which, let me tell you, gives very interesting results. :-)
But its enough for me to test this patch. Everything seems to be working.
Thanks for the patch, Tom! http://drupal.org/cvs?commit=216204
#17
@JohnAlbin: Maybe I was wrong about what caused the symptom, I just know what fixed it.
And anyway, checking whether a constant is defined before using it is better practice.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.
#19
@JohnAlbin Thank you for this useful module.
I'm sorry to reopen this issue but I experiencing problems with my rtl site although I'm using 6.x-1.1 and Tom recent rtl patch
if (defined('LANGUAGE_RTL') && $language->direction == LANGUAGE_RTL)should work.my case:
- site has 2 languages enabled, one ltr and one rtl.
- the site default is the ltr one.
- rtl language negotiation set to path prefix only
- the module should load one of the files, ie6.css or ie7.css if on IE and if language is rtl then load it with it -rtl.css version.
the issue:
If the browser is IE (whether 6 or 7) then the conditional style always load both files (ie6.css and ie6-rtl.css) whether page language is rtl or ltr.
I tried to make the language condition at line 78 to be
$language->direction == LANGUAGE_RTLbut didn't work.I'm out of reasons why this module is working for me.
#20
Update
The issue is clear now
The browser keeps conditional style load both files (ie6.css and ie6-rtl.css) whether page language is rtl or ltr until you "clear cache" from admin>settings>performance.
Once cache cleared you will have no -rtl.css at the ltr . BUT again it gets sticky once you switch to rtl and then it gets back to the first state with no difference (-rtl.css) loads whether on rtl or ltr page until you clear system cache.
drupal core similar functionality adds a dummy query string to keep forcing loads of new copy of files with the url change. http://api.drupal.org/api/function/drupal_get_css/6
$query_string = '?'. substr(variable_get('css_js_query_string', '0'), 0, 1);
#21
RTL cache overwrote LTR cache (or vice-versa)
this patch should fix it.
#22
@aliadnan:
Weird, I haven't seen this behavior in any of the sites I built on Tendu, and never had a bug report about it.
On the other hand I never used "Path Prefix" in any of my sites. Does Drupal cache things differently in this case?
Or maybe you are doing something else like "Aggressive Mode" caching?
In any case, I'll mark the patch posted here as yet another thing I need to add to Tendu when I have the time... thanks.