Posted by druvision on December 29, 2009 at 11:15pm
16 followers
| Project: | Fusion |
| Version: | 7.x-2.x-dev |
| Component: | User interface |
| Category: | bug report |
| Priority: | major |
| Assigned: | Hadi Farnoud |
| Status: | needs work |
| Issue tags: | Acquia Prosper, Fusion, Fusion core, RTL, UberDrupal |
Issue Summary
Many thanks for the effort to RTLise such an imprtant theme.
I've tried to install fusion & acquia prosper with RTL on יש טיפול טבעי, as part of my UberDrupal installation.
However, when I switch to RTL all I get is a mostly white screen. To see it, register to tipul.com, edit your user profite and select the theme.
FYI
Amnon
Comments
#1
Hi Amnon! Thanks for trying it out on your site. We really need folks like you with real RTL sites to help test, as our little testing sites only go so far :)
However, Acquia Prosper isn't yet RTL-themed. There's an issue open to do this and we'll be tackling it soon though: http://drupal.org/node/649184
If you could try out Fusion Starter as your default theme and report back with any issues, that would be extremely helpful.
One thing to note is that the text in the Skinr styles for left/right in blocks will actually be reversed in RTL. ie. the setting that says "Float block to the right" will actually mean left when you're viewing in RTL.
Thanks again!
#2
I was playing today with Fusion Starter on an RTL (hebrew) site I work on.
I'm afraid to say this may be the best RTL/BiDi theme I have worked with, and I am a big fan of Tendu - by far the best starter BiDi theme until Fusion (see comparison table attached here, once published here , look for RTL support).
I work on linux firefox, everything went fine obviously, so I started up my xp VM and tryed IE6, 7 and 8 - at first I was disapointed but then I remembered I didn't have css optimization on, ten seconds later everything was perfect.
The only thing I have noticed unRTLized is the float of -
#content-tabs ul.primary, #content-tabs ul.secondaryand
#content-tabs ul.primary li, #content-tabs ul.secondary libut I know you will fix this soon :)
If I remember tommorow I'll make a patch.
Great job, keep it going.
#3
Issue in comment #2 fixed in Fusion 6.x-1.0-rc1
#4
Automatically closed -- issue fixed for 2 weeks with no activity.
#5
fusion core does not load the gridxx-960.css it just loads gridxx-960-rtl.css
so I added missing bits into rtl ones.
PS: this patch is for Fusion core, should work on any rtl subtheme
#6
sorry about the patches, they were SVN patches. Uploaded again
#7
#8
I have applied this file grid16-960-rtl.css_.patch since it is the only one that I need at my site. Works fine. Thanks very much fo the effort.
#9
#10
Yes. The patches here work and they need to be applied in future releases.
#11
I also checked this patches and it works. please apply in future releases.
#12
#13
The patches provided in #6 just duplicate codes from gridX-Y.css in gridX-Y-rtl.css and don't solve the main problem in a proper way. The problem comes from the following line in the fusion_core_preprocess_html() of the template.php of fusion core theme:
drupal_add_css($path . '/css/' . $file, array('basename' => $name . '-' . $file, 'group' => CSS_THEME, 'preprocess' => TRUE));If you remove basename option from this line like below, everything will work correctly:
drupal_add_css($path . '/css/' . $file, array('group' => CSS_THEME, 'preprocess' => TRUE));It seems that the basename option some how removes gridX-Y.css from the css queue for RTL languages. I wonder if this is a fusion theme bug or drupal 7 core's.
#14
The suggestion inside reply #13 by Massud works fine.
#15
I knew about this by the way, just though a css patch would be easier for others. I will update the patch to reflect your suggestions. the css duplication is not a good solution indeed.
However, some subthemes are made without base css files. they work now but if you apply this patch you probably brake them. It's a bug people got used to in subtheme building. If you apply this patch instead of CSS patches, you may break your theme everytime updating fusion. remember to apply it everytime. Looks like we're not getting a fix by Fusion developer here.
#16
#17
#18
#19
revised
#20
#21
Patch by Hadi Farnoud (#19) works fine inside the new version (version 2)
#22
I'm not quite sure I understand the implications of this patch. Fusion is assigning a basename equal to the name of the theme (eg: "fusion_core") + the name of the grid file (eg: "grid12-960.css"). That should make each file called by drupal_add_css() to have a unique basename across themes, even if the same filename (eg: "style.css") is used.
The doc on the basename option isn't very clear to me, but I take it this is some form of namespacing?
What I don't understand is why namespacing is required if the two node.css files in the above example are in distinct locations. I also don't understand the term "basename" in this context, or how it's used. In PHP, the basename() function returns the trailing name component. I don't know now that relates to adding CSS files.
You folks seem to have a better understanding, so any tips would be helpful.
If the basename is causing incompatibilities with RTF, that does sound like a potential bug in core and the issue should be filed there to rule that out.
#23
As #13 said, basenamejust makes the naming convention wrong for some reason. if you try it with an RTL language like Farsi, you'll see the css is completely messed up.
This might not matter to LTR languages. But for RTL languages (which are many) is a serious bug! Please get this bug the attention it deserves.
#24
Hi Hadi, the proposed patch could have implications on thousands of sites running Fusion, so I need a clearer understanding of why the patch in #19 fixes the issue. "makes the naming convention wrong for some reason" doesn't provide much information. Why is basename causing problems? What affect will removing it create for other sites?
#25
Thank's for that.
Patch in #19 works grate for me.
Should be committed, no?
#26
I changed the line manually in fusion_core/template.php and it solved the issue, thanx Hadi.
However, I tested it as a patch and it did not work in my case.
I never tried to patch drupal before so it is most likely my bad, what I did is put the patch file in sites/all/themes/fusion , and run the following:
patch -p0 --dry-run patch_file_name.patchIt did not output anything, also never returned me to the prompt.
tried it also from drupal base dir and got the same, didn't try without the dry-run.
Only reason I bother this list by reporting this, is because I see recent activity here so - just in case I save someone a few minutes.
As I said, the suggested fix did work.
#27
Can anyone answer the question that aquariumtap posed in #24? In the very least, any assistance we could have in recreating and testing this issue for ourselves would be very helpful. If anyone has a multi-lingual demo site that presents this RTL issue and can share the code and database with us, it would help us understand the issue, test the patches and get this fix into a release.
Thanks!
#28
patch #19 worked partially
And the answer for why it worked that because using "drupal_add_css" with "basename" is to force basename in case of multiple modules with similar names and since "drupal_add_css" called with full path name so there no place for other css files to get loaded.
also in the following code from drupal show why "basename" is not needed if we provide "filename" ($item['data'])
function drupal_get_css($css = NULL, $skip_alter = FALSE) {
...
$previous_item = array();
foreach ($css as $key => $item) {
if ($item['type'] == 'file') {
// If defined, force a unique basename for this file.
$basename = isset($item['basename']) ? $item['basename'] : basename($item['data']);
#29
#19 fixes the issue #28 explained. I tested this patch on both English and Farsi version. Didn't break mine. I don't think it does any harm to existing themes. Can anyone else test and report back?
#30
this patch for beta2 I test it in English and Arabic.
#31
#30 patch works for English and Arabic Languages.
#32
You can test it here
http://tech-news.dreamswebsite.com
#33
#30: CSS_RTL_fix_beta-2.patch queued for re-testing.
#34
path #19 works for master branch.
Thank you Hadi Farnoud
#35
#36
#37
Hi Hadi,
I'd love to get this into Fusion for you but can you help clarify? I see a patch against beta2 in two places (#19 and #30) - Could you possibly regenerate this patch for the 7.x-2.x-DEV version? If you can do that, I'll test it and we'll get this into Fusion ASAP.
Thanks for your work!
#38
Also moving this into DEV version.