Issue Details (XML | Word | Printable)

Key: MDL-10075
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Blocker Blocker
Assignee: Urs Hunkler
Reporter: Urs Hunkler
Votes: 0
Watchers: 3
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

Can't add rtl css files. Does Moodle load theme/config.php several times with different $CFG set?

Created: 11/Jun/07 12:46 AM   Updated: 30/Jul/07 04:51 AM
Return to search
Component/s: Themes
Affects Version/s: 1.9
Fix Version/s: 1.9

File Attachments: 1. Text File rtl_styles.patch (4 kB)
2. Text File rtl_theme.patch (8 kB)

Environment: Server: UBUNTU Linux, 2.6.17-11, apache 2, PHP Version 5.1.6
Issue Links:
Blockers
 
Dependency
 
Relates
 

Participants: Mauno Korpelainen, Petr Skoda and Urs Hunkler
Security Level: None
Resolved date: 30/Jul/07
Affected Branches: MOODLE_19_STABLE
Fixed Branches: MOODLE_19_STABLE


 Description  « Hide
Moodle does not load the 'rtl' CSS files.

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Urs Hunkler added a comment - 11/Jun/07 12:53 AM
1) I added central var langdirection - is in CVS
set $CFG->langdirection in lib/setup.php line 590
$CFG->langdirection = (get_string('thisdirection') == 'rtl') ? 'rtl' : 'ltr';

2) I added body CLASS for langdirection - is in CVS
add body class in lib/weblib.php - print_header line 2455
if (!empty($CFG->langdirection)) { $pageclass .= ' ' . $CFG->langdirection; }

3) I select theme sheets depending on $CFG->langdirection (debugging included) - is in CVS
trigger_error('custom_corners/config.php - $CFG->langdirection: '.$CFG->langdirection, E_USER_NOTICE);

if ($CFG->langdirection == 'rtl') {
$THEME->sheets = array('user_styles', 'styles_rtl');
trigger_error('custom_corners/config.php - is rtl ::: ', E_USER_NOTICE);
} else {
$THEME->sheets = array('user_styles');
trigger_error('custom_corners/config.php - is not rtl ::: ', E_USER_NOTICE);
}

trigger_error('custom_corners/config.php - $THEME->sheets: '.$THEME->sheets[0].', '.$THEME->sheets[1], E_USER_NOTICE);

1 and 2 work perfectly.

3 does not work. Moodle always uses the ltr sheets.

The set of debug messages is written to the error log 4 or more times. The first time correctly with the right langdirection. All following message sets war wirtten with the wrong langdirection. Does this mean that Moodle loads the theme/config.php several times and with different global variables?


Urs Hunkler added a comment - 11/Jun/07 12:56 AM
I need urgent help!

I tired to understand the process 2 days long, but I could not find any starting point to understand why the config.php seams to be loaded several times.

How does Moodle handle the theme building? Might I have found a bug?


Urs Hunkler added a comment - 12/Jul/07 09:00 PM
Martin (or Petr), are you interested in true rtl support? If yes, please help!!!

Mauno Korpelainen added a comment - 20/Jul/07 06:57 PM - edited
Urs,

just one thought: is it possible to have the code in styles.php of language folders because this problem is only connected to rtl languages?
You already have a possibility to set $THEME->langsheets = true; in theme config.php and to include styles.php inside rtl language folders - or simply have different code for ltr languages styles.php and rtl languages styles.php.


Urs Hunkler added a comment - 20/Jul/07 10:03 PM
Mauno, the langsheets might be an emergency solution if nothing else works.

Themes will need a longer documentation where users need to copy which files. That is contra productive. Therefore I prefer a solution within the theme logic. And I don't understand why my approach is not working.

Probably we will have to wait for an answer until Moodle enters the User Experience market maturity stage


Petr Skoda added a comment - 23/Jul/07 07:10 AM
I do not think this could work, the problem is that the theme css files are included from style.php which does not known the correct language (no course_setup()).

Solution could be to use one sheet for both rtl and ltr languages - see attached path, seems to work fine for ar lang pack here (with one minor corner bug on the frontpage)


Mauno Korpelainen added a comment - 23/Jul/07 05:10 PM
Yes, a single stylesheet works ok. I tested one version in http://tracker.moodle.org/browse/MDL-9977 and the main problem was to set paddings and margins properly for all browsers. It may be easier to create new themes with three images for box top and three images for box bottom instead of one bt and one bb image.

Urs Hunkler added a comment - 24/Jul/07 01:30 AM
I got a solution: Looking at the langsheet handling I found out, that lang info is handled via parameters in the url.

I added a langdir parameter in weblib and add it to the url in styles.php.

This way one can define any file in any theme with '_ltr' or '_rtl' postfixes and Moodle will filter out the wrong one. And this way theme designers can decide if they want to use alternative CSS files for ltr and rtl positioning. This is the preferable way because we keep the CSS downloaded as small as possible.

Or they may write ltr only CSS and add one rtl CSS file which overwrites the important definitions.

Just add the CSS files for both directions in the theme config file. Moodle filters out the "wrong" ones.

We need such a solution because transferring ltr and rtl formatting in one file makes the CSS much larger. And Moodle CSS is already that large that I prefer to not add any unnecessary byte. Most programmes I know offering rtl support solve the issue with separate ltr and rtl CSS files which are selected depending on the language writing direction.

Petr, are there any concerns using this method?


Petr Skoda added a comment - 24/Jul/07 03:29 AM
Minor problem might be with caching, I would recommend using optional_param with default ltr.

Do you need any help with coding?


Urs Hunkler added a comment - 30/Jul/07 04:51 AM
Petr, thanks for offering your help. Going this way there where only two small changes and some cleaning up to do. "MDL-10075 - changes done following Petr's patch and optional_param with default ltr".