962061 - Fix the footer RTL layout (language switcher: ltr direction is hard-coded)

Status: RESOLVED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Francesco Lodolo [:flod]]

I believe the language switcher is not displayed correctly right now in the footer for RTL languages.
http://www.mozilla.org/ar/

From right to left I see the select before the label, while it should be the opposite (CCing one of our Arabic localizers to confirm).

ltr is hardcoded in both form and select, so it should be enough to replace it with {{ DIR }} or remove it completely. Not sure if there's a reason for the current code though.

https://github.com/mozilla/bedrock/blob/master/bedrock/base/templates/includes/lang_switcher.html#L6

Comment 1

[Kohei Yoshino [:kohei]]

Hmm, actually the entire footer is not correctly in RTL langs, I think. Will take a look.

Comment 2

[Kohei Yoshino [:kohei]]

I meant: not *displayed* correctly

Comment 3

[Youghourta Benali]

Indeed, the entire footer is not correctly displayed in the Arabic homepage, but  the label is displayed before the select ( as expected): 
http://imgur.com/2fiScY4

Comment 4

[Francesco Lodolo [:flod]]

Attachment: Screenshot on Mac

Interesting, on Mac it's quite different. Using "rtl" instead of "ltr" reverses the order but still on one line.

Same footer on the Italian page, label and select are on two lines.

Comment 5

[Kohei Yoshino [:kohei]]

Attachment: Pull Request on GitHub

Comment 6

[Kohei Yoshino [:kohei]]

Attachment: screenshot, postfix

Here's a post-fix screenshot on Firefox. I'm still wondering if the copyright year notation is correct. It reads "2014–1998©". Should we encourage RTL localizers to code like this?

> <span dir="ltr">©1998–%(current_year)s</span>

Comment 7

[Francesco Lodolo [:flod]]

Interesting question. Why isn't the text direction reversed like the rest? Can we fix that at replacement level instead of asking localizers to do it?

Comment 8

[Youghourta Benali]

The text in the select should be aligned to the right too, right?

where can I find the copyright string (which lang file)? May be we need to fix the string itself

Comment 9

[Francesco Lodolo [:flod]]

The copyright string is in main.lang, and you're right about the select (commented on Github).

Comment 10

[Francesco Lodolo [:flod]]

It it just me or we don't have Arabic as a choice in the language switcher? Should I file another bug or can we use this?

Comment 11

[Youghourta Benali]

Me neither, I don't have Arabic in the language switcher (contribute page)

Comment 12

[Kohei Yoshino [:kohei]]

(In reply to Francesco Lodolo [:flod] from comment #7)
> Interesting question. Why isn't the text direction reversed like the rest?
> Can we fix that at replacement level instead of asking localizers to do it?

It can be:

> {% trans url=url('foundation.licensing.website-content'),
>          copyright='<span dir="ltr">©1998–%d</span>'|replace('%d', current_year)|safe %}
> Portions of this content are {{ copyright }} by individual
> mozilla.org contributors. Content available under
> a <a href="{{ url }}">Creative Commons license</a>.
> {% endtrans %}

(In reply to Youghourta Benali from comment #8)
> The text in the select should be aligned to the right too, right?

Hmm, but locale names are not localized, and dir="rtl" on <select> breaks labels including brackets. I thought we could keep it.

(In reply to Youghourta Benali from comment #11)
> Me neither, I don't have Arabic in the language switcher (contribute page)

The contribute page is not localized into Arabic yet.

Comment 13

[Francesco Lodolo [:flod]]

(In reply to Kohei Yoshino [:kohei] from comment #12)
> Hmm, but locale names are not localized, and dir="rtl" on <select> breaks
> labels including brackets. I thought we could keep it.
I don't think that's a problem if names are in a different locale, it probably looks strange for RTL users.
 
> The contribute page is not localized into Arabic yet.
Didn't think about that. Checked on stage and it's working correctly.

Comment 14

[Kohei Yoshino [:kohei]]

How about using the dir and lang attributes on each <option> like this:

<option value="ar" lang="ar" dir="rtl">عربي</option>
<option value="en-US" lang="en-US" dir="ltr">English (US)</option>

Comment 15

[Francesco Lodolo [:flod]]

Does it work? I think arrow and text remain on the wrong side.

Comment 16

[Kohei Yoshino [:kohei]]

Yeah, I think it's better.

<option dir="{% if code in ['ar', 'fa', 'he'] %}rtl{% else %}ltr{% endif %}">

Comment 17

[Youghourta Benali]

Could you please replace
عربي
by
العربية

thanks

Comment 18

[Kohei Yoshino [:kohei]]

(In reply to Youghourta Benali from comment #17)
> Could you please replace
> عربي
> by
> العربية

This locale label is not in the footer code. It's used site-wide... Is this the source?
http://viewvc.svn.mozilla.org/vc/libs/product-details/localeDetails.class.php?view=markup#l22

Comment 19

[Francesco Lodolo [:flod]]

Pascal, any idea about this? ^^

I guess we should split the bug to fix that.

Comment 20

[Pascal Chevrel:pascalc (PTO until April 26)]

(In reply to Kohei Yoshino [:kohei] from comment #16)
> Yeah, I think it's better.
> 
> <option dir="{% if code in ['ar', 'fa', 'he'] %}rtl{% else %}ltr{% endif %}">

please add ur (Urdu) to the list, we don't ship it but it's a potential future rtl locale. thanks

Comment 21

[Pascal Chevrel:pascalc (PTO until April 26)]

(In reply to Kohei Yoshino [:kohei] from comment #18)
> (In reply to Youghourta Benali from comment #17)
> > Could you please replace
> > عربي
> > by
> > العربية
> 
> This locale label is not in the footer code. It's used site-wide... Is this
> the source?
> http://viewvc.svn.mozilla.org/vc/libs/product-details/localeDetails.class.
> php?view=markup#l22

Yes this is the source, it is also used on download buttons, so any change to the string must also work for the use on the download button. Looking at Firefox source code which was probably the original source, both these strings exists in different context:
http://transvision-beta.mozfr.org/?recherche=arabic&repo=release&sourcelocale=en-US&locale=ar&search_type=strings

So no problem for me if we change that (in a different bug yes) as long as it is not a problem for the context of the download button.

Comment 22

[Kohei Yoshino [:kohei]]

(In reply to Pascal Chevrel:pascalc from comment #20)
> please add ur (Urdu) to the list, we don't ship it but it's a potential
> future rtl locale. thanks

I've replaced the list with settings.LANGUAGES_BIDI. Looks like Urdu (ur) is not included in settings.LANGUAGES_BIDI in the current Django version used by Bedrock. It will be enabled once Django is upgraded.
https://github.com/django/django/blob/master/django/conf/global_settings.py#L135

Comment 23

[[github robot]]

Commits pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/5e9985b35161d45e006a6f3a2de8ac18a9e21d02
Fix Bug 962061 - Fix the footer RTL layout (language switcher: ltr direction is hard-coded)

https://github.com/mozilla/bedrock/commit/6b3b1bfad294789f43427c50cb5814a75cd34043
Merge pull request #1606 from kyoshino/bug-962061-footer-rtl

Fix Bug 962061 - Fix the footer RTL layout (language switcher: ltr direction is hard-coded)