Opened 8 years ago

Last modified 6 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).



Subtickets

Attachments (1)

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

Download all attachments as: .zip

Change History (20)

comment:1 Changed 8 years ago by Will Kahn-Greene

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

comment:2 Changed 7 years ago by Christopher Allan Webber

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 Changed 7 years ago by Christopher Allan Webber

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 Changed 7 years ago by Jiyda Mint Moussa

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 Changed 7 years ago by Jiyda Mint Moussa

Keywords: needs-review added

comment:6 Changed 7 years ago by Christopher Allan Webber

Owner: Osama Khalid deleted
Status: acceptedreview

comment:7 Changed 7 years ago by Christopher Allan Webber

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 Changed 7 years ago by Christopher Allan Webber

Cc: osamak added

comment:9 Changed 6 years ago by Aleksej

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 6 years ago by Aleksej (previous) (diff)

comment:10 Changed 6 years ago by Genghis Khan

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 Changed 6 years ago by Christopher Allan Webber

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?

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

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 Changed 6 years ago by Christopher Allan Webber

Resolution: fixed
Status: reviewclosed

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 Changed 6 years ago by Genghis Khan

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 Changed 6 years ago by Genghis Khan

Resolution: fixed
Status: closedaccepted

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 Changed 6 years ago by Genghis Khan

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

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

comment:17 Changed 6 years ago by Genghis Khan

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

I hope there are no other locales left.

comment:18 Changed 6 years ago by Christopher Allan Webber

Keywords: needs-review removed

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

comment:19 Changed 6 years ago by Genghis Khan

What does bitesized mean?

Note: See TracTickets for help on using tickets.