After enabeling the language module I was able to edit the english language and make it "right to left".
But the "right to left" stylsheets weren't added to the source.
I noticed that the rtl stylesheet system only works when locale is enabled.

This sounds like a major WTF. We shouldn't add options in one module if they don't work without enabling another one.

CommentFileSizeAuthor
#8 1405604-final-fix.patch3.04 KBaspilicious
#6 1405604-should-fail.patch671 bytesaspilicious
#1 1405604-rtl-stylesheets-1.patch2.59 KBaspilicious
#1 1405604-rtl-stylesheets-1b.patch3.05 KBaspilicious

Comments

aspilicious’s picture

aspilicious commented
StatusFileSize
new 1405604-rtl-stylesheets-1b.patch3.05 KB
new 1405604-rtl-stylesheets-1.patch2.59 KB

first should fail, second one will pass

aspilicious’s picture

aspilicious commented
Status: Active » Needs review
aspilicious’s picture

aspilicious commented

I added an extra newline (by accident), will remove that in a next reroll

xjm’s picture

xjm
she/her
Primary language English
commented

Awesome! Nice simple fix. Going to wait and see what the bot is, though. This seems borderline major, but @aspilicious and I decided to check with Gabor.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs tests

Hmm, the version without the test fix also passed. Looks like we do need more test coverage.

aspilicious pointed out that locale requires language; duh. So the test passed anyway.

aspilicious’s picture

aspilicious commented
StatusFileSize
new 1405604-should-fail.patch671 bytes

Locale requires language so 1 en 1B will always succeed...

I guess this new file should fail...

Status: Needs review » Needs work

The last submitted patch, 1405604-should-fail.patch, failed testing.

aspilicious’s picture

aspilicious commented
Status: Needs work » Needs review
StatusFileSize
new 1405604-final-fix.patch3.04 KB

Ok, now I'm happy.
So this patch is the same as 1b without the extra newline.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +D8MI

Good catch. This was certainly a miss when the language related code was moved to language.module. Looks good, it is just a 1-1 move of a hook.

BTW Another related issues where we are making node and path module closer to rely only on language module for their language services (vs. locale module) is #1387608: Unify language_list() and locale_language_list(), help welcome :)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Looks good and was clearly an oversight in an earlier patch. Committed to 8.x. Thanks.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +language-base

Tagging for base language system.

Automatically closed -- issue fixed for 2 weeks with no activity.