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.
Comment | File | Size | Author |
---|---|---|---|
#8 | 1405604-final-fix.patch | 3.04 KB | aspilicious |
#6 | 1405604-should-fail.patch | 671 bytes | aspilicious |
#1 | 1405604-rtl-stylesheets-1.patch | 2.59 KB | aspilicious |
#1 | 1405604-rtl-stylesheets-1b.patch | 3.05 KB | aspilicious |
Comments
Comment #1
aspilicious CreditAttribution: aspilicious commentedfirst should fail, second one will pass
Comment #2
aspilicious CreditAttribution: aspilicious commentedComment #3
aspilicious CreditAttribution: aspilicious commentedI added an extra newline (by accident), will remove that in a next reroll
Comment #4
xjmAwesome! 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.
Comment #5
xjmHmm, 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.
Comment #6
aspilicious CreditAttribution: aspilicious commentedLocale requires language so 1 en 1B will always succeed...
I guess this new file should fail...
Comment #8
aspilicious CreditAttribution: aspilicious commentedOk, now I'm happy.
So this patch is the same as 1b without the extra newline.
Comment #9
Gábor HojtsyGood 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 :)
Comment #10
Dries CreditAttribution: Dries commentedLooks good and was clearly an oversight in an earlier patch. Committed to 8.x. Thanks.
Comment #11
Gábor HojtsyTagging for base language system.