Opened 13 years ago

Last modified 11 years ago

#220 accepted defect

RTL (right-to-left) language support.

Reported by: Christopher Allan Webber Owned by:
Priority: minor Milestone:
Component: programming Keywords: bitesized
Cc: osamak Parent Tickets:

Description (last modified by Christopher Allan Webber) 

We need RTL (right-to-left) language support in MediaGoblin.

Presumably this requires:


-  Pushing a variable into the general template context specifying
   whether the current language is LTR or RTL
-  Making an RTL stylesheet
-  ... other things?

I'm assigning Osama Khalid to this ticket. Osama, you don't have to
do this necessarily, but at least please give advice on what needs
to be done (you can unassign yourself then if you like).

Attachments (1)

0003-is_rtl-recognizes-country-specific-varieties-of-RTL-.patch​ (1.1 KB ) - added by Leah Velleman 9 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 by Will Kahn-Greene, 13 years ago

The original url for this bug was ​http://bugs.foocorp.net/issues/522 .

comment:2 by Christopher Allan Webber, 12 years ago

Component: → programming
Description: modified (diff)
Keywords: bitesized added

This might not be too complex for someone who knows how to do it, but I'm not sure. I think this is mostly CSS work, in which case this would be bitesized, so marking it as such...

comment:3 by Christopher Allan Webber, 12 years ago

So, I think we probably want to do a few things:

  • first of all, it looks like neither babel nor python come with a list of which languages are RTL. We could probably provide a list of which languages are RTL in mediagoblin/tools/translate.py and then add a utility like is_rtl ... probably like so: 
# Known RTL languages
KNOWN_RTL = set(["ar"])

def is_rtl(lang):
    return lang in KNOWN_RTL
  • add this function is_rtl to the global context. The way to do this is to open mediagoblin/tools/template.py in this section: 
    template_env.globals['fetch_messages'] = messages.fetch_messages
    template_env.globals['app_config'] = mg_globals.app_config
    template_env.globals['global_config'] = mg_globals.global_config
    template_env.globals['version'] = _version.__version__

Add something there for is_rtl which should be the function mentioned above.

  • I think maybe various elements could then be marked for possible rtl depending on the language, and an RTL stylesheet could then be included conditionally?

That's my guess on best way to approach this :)

comment:4 by Jiyda Mint Moussa, 12 years ago

Here is the branch and the commit:
​https://gitorious.org/~jiyda/mediagoblin/jiydas-mediagoblin/commit/4a4e4e4ae83d6ff3f09580238049e74444b01416

I added dir="rtl" to the head element and added a div just inside the body element with the same attribute when the language of the locale is right-to-left.
The reason I used the dir attribute is because w3 recommend not to use CSS styling for direction (​http://www.w3.org/TR/i18n-html-tech-bidi/#ri20030728.092130948.)

comment:5 by Jiyda Mint Moussa, 12 years ago

Keywords: needs-review added

comment:6 by Christopher Allan Webber, 12 years ago

Owner: Osama Khalid removed
Status: accepted → review

comment:7 by Christopher Allan Webber, 12 years ago

This looks good generally. There are some issues I think (though Jiyda, you did a great job and did exactly as requested! :)):

  • Maybe it RTLs too much? For example, suddenly thumbnails are right-aligned.
  • I actually don't know what's the ideal way to handle a description written in English, etc, but suspect it won't be hndled in this patch

I'm tempted though to maybe merge it, and maybe the frustration of it not working totally right means that some RTL-using communities will be able to help us iterate through something better? (Maybe I could even close this ticket and it could be reopened with more advice if needed?)

At the very least, jiyda did a great job and implemented what was asked. Should this be merged?

comment:8 by Christopher Allan Webber, 12 years ago

Cc: osamak added

comment:9 by Aleksej, 12 years ago

On the need to specify direction (whether LTR or RTL): ​https://aharoni.wordpress.com/2012/08/07/always-dir-01/

On specifying the wrong direction (expecting text to be translated when it is not, thus RTLing too much): ​https://aharoni.wordpress.com/2013/05/18/always-define-the-language-and-the-direction-of-your-html-documents-part-02-backwards-english/

Last edited 12 years ago by Aleksej (previous) (diff)

comment:10 by Genghis Khan, 11 years ago

For start, we can modify </body> to adjust to RTL languages and the rest of the issues, if there would be any, may bee solved later.

  • <unnamed>

     
    3131
    3232    <link rel="alternate" type="application/atom+xml" href="/atom/">
    3333  </head>
    34   <body>
     34  <body style="direction: rtl">
    3535   
    3636   
    3737     

RTL languages may be listed in an external list by developers, or within translation files by translators, as GTK+ and Qt do.

comment:11 by Christopher Allan Webber, 11 years ago

If putting it in the style: is all, that's easy. My main concern there was that this would mess up other important things' alignment.

I just gave it a test, a couple of things do get goofed up, but they're a pretty small amount. But there are definitely some places we are rtl'ing too much if that gets flipped, as per aleksej's comment. Maybe we could fix all of those individually?

I'm still not sure how to handle "this comment is LTR" and "this text is RTL", but maybe ​https://developer.mozilla.org/en-US/docs/Web/CSS/unicode-bidi is the right route for that?

Should I just merge jidya's patch and make some adjustments, and we move forward from there then?

in reply to:  11 comment:12 by Genghis Khan, 11 years ago

Replying to cwebber:

If putting it in the style: is all, that's easy. My main concern there was that this would mess up other important things' alignment.

The biggest issue I think is going to be is profile description which will be very easy to fix by copying RTL character recognition from ​Gajim's, which is by far the best I know, and maybe some little Javascript code, for time when user is writing text, from Transifex. I would like to propose on solving further issues after trying out next GMG release with jidya's patch.

I just gave it a test, a couple of things do get goofed up, but they're a pretty small amount. But there are definitely some places we are rtl'ing too much if that gets flipped, as per aleksej's comment. Maybe we could fix all of those individually?

Yes, no problems.

Should I just merge jidya's patch and make some adjustments, and we move forward from there then?

Yes, please do. Each other issue will be solved at a time.

comment:13 by Christopher Allan Webber, 11 years ago

Resolution: → fixed
Status: review → closed

Well then. Merged! We'll continue to deal with whatever issues as they come up then.

Thanks everyone who helped this land, especially jiyda, whose branch I just landed in master! Yay!

comment:14 by Genghis Khan, 11 years ago

Right-to-left works great!

I could not add a subticket so I have opened a new related issue of a tiny problem at #828.

comment:15 by Genghis Khan, 11 years ago

Resolution: fixed
Status: closed → accepted

Problem Please also include Persian/Iran (fa-ir) locale and Arabic locales such as Arabic/Syria (ar-sy) and the remain of the ar-* locales.

comment:16 by Genghis Khan, 11 years ago

Add ar-* and fa-* for Persian/Afghanistan (fa-af).

Last edited 11 years ago by Genghis Khan (previous) (diff)

comment:17 by Genghis Khan, 11 years ago

Add ur-* for Urdu (ur), Urdu/India (ur-in) and Urdu/Pakistan (ur-pk).

I hope there are no other locales left.

comment:18 by Christopher Allan Webber, 11 years ago

Keywords: needs-review removed

This would be a nice bitesized ticket to wrap up adding those locales.

comment:19 by Genghis Khan, 11 years ago

What does bitesized mean?

by Leah Velleman, 9 years ago

Attachment: 0003-is_rtl-recognizes-country-specific-varieties-of-RTL-.patch​ added
Note: See TracTickets for help on using tickets.