Ok, so I have a patch in bug9157 on https://github.com/sysrqb/bridgedb
It's hacky, but I think until someone else can spend time to do this correctly, this will do. Screenshots of it running locally are attached.
So, there are a couple things which need a bit of touchup:
In HTTPServer.py, in classes WebRoot and WebResource, you do:
{{{
# Grab only the language (first two characters) so we know if we need
# to right-justify the text
langs = [ lang[:2] for lang in langs ]
rtl = False
if "ar" in langs or "fa" in langs:
rtl = True
}}}
which, for example, if someone were to speak Deutsch and Arabic, they would see German text going right-to-left. See attached screenshot.
There is a lot of duplicate code...the above bug is duplicated twice. The following structure is replicated several times:
{{{
% if rtl:
% else:
% endif
${_("My bridges don't work! I need help!")}
}}}
which also brings me to...
#2 (closed) is a bug also. This ticket was ill-specified, and it sucks a bit that you put work into it before someone could point out that right-alignment wasn´t what was needed. Farsi and Arabic (and others) need to be written right-to-left, which isn't the same as align-right, it's more like mirror image. I can read small amounts of Arabic, and the first thing I noticed was that the punctuation is on the wrong side of the sentence. Doing this can be a bit tricky to get right, esp. in our case where we have English text in the middle of RTL languages. The W3C has some good [http://www.w3.org/International/tutorials/bidi-xhtml/ documents] on best practices for getting it right, IIRC there's something about using the dir tag in the <meta> sections to set the default, and only when you need to change directions to not-the-default it gets used again. Also, I am not a web developer either, perhaps someone else knows more. In fact, I'm sure one of our friends on IRC coming from an RTL-language speaking country could probably write this code in five minutes, or, at least, better point out when things look weird to them.
Farsi and Arabic aren't the only languages which we support which are RTL, off the top of my head, we've already got translations for Hebrew, and Kurdish is also one of the languages we support (if you want a wikipedia blackhole and an interesting history lesson about the mistreated Kurdish ethnic groups in Iran, Turkey and Syria...that just ate my past ten minutes). See the bridgedb* branches in [https://gitweb.torproject.org/translation.git the translation repo] for all the languages we get translations for.
Line wrapping. This one also isn't your fault, because the line was already that way, but the OCD side of me prefers that PEP8 be followed when touching lines of code (which, yes, means fixing up past contributors mistakes). For example, I am looking at L16 in lib/bridgedb/i18n/bridges.html -- it's like a zillion columns long! In the case of translations, this is actually a tiny bit more than OCD, as someone has to review all the .po files from the translator volunteers before merging (mostly to make sure there aren't any
5) Line wrapping. This one also isn't your fault, because the line was already that way, but the OCD side of me prefers that PEP8 be followed when touching lines of code [...]
So, there are a couple things which need a bit of touchup:
In HTTPServer.py, in classes WebRoot and WebResource, you do:
{{{
# Grab only the language (first two characters) so we know if we need
# to right-justify the text
langs = [ lang[:2] for lang in langs ]
rtl = False
if "ar" in langs or "fa" in langs:
rtl = True
}}}
which, for example, if someone were to speak Deutsch and Arabic, they would see German text going right-to-left. See attached screenshot.
True, I wanted to limit it to the first lang in the Languages header. The issue I had was that we have no guarantee that we support the first language and we can't (easily) query gettext, as far as I can tell. So, presently I hope everyone prefers languages read/written rtl over all others.
2) There is a lot of duplicate code...the above bug is duplicated twice.
Yeah, this certainly isn't the best implementation. I was/am more concerned with not having a crappy website than code quality, specifically for this.
3) #2 is a bug also. This ticket was ill-specified, and it sucks a bit that you put work into it before someone could point out that right-alignment wasn't what was needed. Farsi and Arabic (and others) need to be written right-to-left, which isn't the same as align-right, it's more like mirror image.
Initially I thought you were wrong, however now it appears I was wrong, indeed. The answer was to add the direction css attribute. Now all is good(er). I expected the translation to be typed rtl, so I thought the only issue was that the text was not right-justified (which this does a hacky job of accomplishing). hrmph. This should be correct now.
I can read small amounts of Arabic, and the first thing I noticed was that the punctuation is on the wrong side of the sentence. Doing this can be a bit tricky to get right, esp. in our case where we have English text in the middle of RTL languages. The W3C has some good documents on best practices for getting it right, IIRC there's something about using the dir tag in the <meta> sections to set the default, and only when you need to change directions to not-the-default it gets used again. Also, I am not a web developer either, perhaps someone else knows more. In fact, I'm sure one of our friends on IRC coming from an RTL-language speaking country could probably write this code in five minutes, or, at least, better point out when things look weird to them.
Probably true.
4) Farsi and Arabic aren't the only languages which we support which are RTL, off the top of my head, we've already got translations for Hebrew, and Kurdish is also one of the languages we support (if you want a wikipedia blackhole and an interesting history lesson about the mistreated Kurdish ethnic groups in Iran, Turkey and Syria...that just ate my past ten minutes). See the `bridgedb*` branches in [the translation repo](https://gitweb.torproject.org/translation.git) for all the languages we get translations for.
Right, this was just me taking this ticket very literally. That was very dumb, I admit.
5) Line wrapping. This one also isn't your fault, because the line was already that way, but the OCD side of me prefers that PEP8 be followed when touching lines of code (which, yes, means fixing up past contributors mistakes). For example, I am looking at L16 in lib/bridgedb/i18n/bridges.html -- it's like a zillion columns long! In the case of translations, this is actually a tiny bit more than OCD, as someone has to review all the .po files from the translator volunteers before merging (mostly to make sure there aren't any <script> tags or other hijinks which could be used to exploit a browser). If the line is a zillion columns long, there's a higher chance that I might scroll past it and not actually see the end of it, especially if I'm bored from looking at 20+ files of gibberish.
Yes, that is quite a zillion columns longs (for some number 'zillion' consisting of ~176 characters)! :p
(I also didn't whitespace-check it, don't shoot me!)
Don't worry, I don't shoot accomplices. I just incessantly nag them about obsessive compulsive formatting divergences. :)
Overall, this looks really good. One thing, and it's not that important, because overall the documentation is great, but just so that you don't have to dig through the Sphinx API documentation on parameter specifications...the type of a :param: directive is specified like this:
:param str beach: The place with sand where you go to code and surf.
or like this if it has some weird type (like it's a class or something):
:type beach: :class:`sys.env.Beach`:param beach: The place with sand where you go to code and surf.
:returns: is the same:
:rtype: str:returns: Some string.
or
:rtype: :class:`sys.env.Beach`:returns: That place with sand where you go to hack.
Also, I totally don't care for now, since I'm sure Sphinx would choke on a lot worse if I tried to generate docs from BridgeDB's code right now. :P
Trac: Status: needs_review to accepted Owner: N/Ato isis
I cherry-picked the above-mentioned commit a31c592e into my branch 'testing/sysrqb/bug9157_r2_rebase-cherrypick-a31c592e', and it seems to be working great. Screenshot attached for Farsi, and it even handles putting the keybinding tags on the correct side of the links for my I-hate-using-a-mouse extension. :D
I remember testing a couple days later from my branch testing/sysrqb/bug9157_r2_rebase-cherrypick-a31c592e (which is what was merged for deployment) and noticing that even though several variants of en are sent with top-priority in my headers, that because I accept multiple languages, some of which are RTL, that bridgedb responded with English going right-to-left. Which was cool looking, but if that is the case, we should sort that out before deploying. Someone should test again from the above branch and see if they get the same thing.
Trac: Resolution: fixed toN/A Status: closed to reopened
Hm, can you retest and see if you can reproduce it? I tested with English, Arabic, Hebrew (in that order) and it still rendered LTR. Let me know how you break it, so that I can fix it. :)
Hm, can you retest and see if you can reproduce it? I tested with English, Arabic, Hebrew (in that order) and it still rendered LTR. Let me know how you break it, so that I can fix it. :)
I can reproduce the bug if I test your patches on top of the new translations that I'm merging, but not if I test your patches on top of master -- so I suspect it is a problem with the translation files.
What is happening is actually that English isn't displaying at all...instead it returns the first non-English language in the Accept-Language list. Here's a screenshot, running from your patches on top of the newly merged translations, requesting three types of English, then French (the red text is the relevant log lines of the request, it's not actually on the page).
I am inclined to believe that the newly merged translations are messing this up, not your patches. And, obviously, the text direction is fine for French, so I'm closing this as fixed and possibly opening a new ticket for why English, of all things, seems to be missing.
Trac: Resolution: N/Ato fixed Status: needs_information to closed