Style sheets order when using RTL language [#1372170]

I think it happened in #228818: IE: Stylesheets ignored after 31 link/style tags but I guess there was no point in reopening that old and closed issue.
What happens is that RTL style sheets get @imported in the end of the group and not immediately after their LTR counterpart.
that means that if you have a parent theme and a sub-theme and they have parent.css, parent-rtl.css, child.css, child-rtl.css respectively.
you would expect :

<style type="text/css" media="all">
@import url(" ... /parent.css");
@import url(" ... /parent-rtl.css");
@import url(" ... /child.css");
@import url(" ... /child-rtl.css");
</style>

but instead you would get :

<style type="text/css" media="all">
@import url(" ... /parent.css");
@import url(" ... /child.css");
@import url(" ... /parent-rtl.css");
@import url(" ... /child-rtl.css");
</style>

This way there is no for a themer to override parent-rtl.css from child.css. There are work-arounds but this feels like a bug.

Comment	File	Size	Author
#1	rtl-overrides-weights-1372170.patch	593 bytes	tsi
#1	D7-rtl-overrides-weights-1372170.patch	555 bytes	tsi

Comments

Comment #1

tsi commented 1 November 2012 at 11:41

Status:

Active

» Needs review

Status	File	Size
new	D7-rtl-overrides-weights-1372170.patch	555 bytes
new	D8-rtl-overrides-weights-1372170.patch	593 bytes

Stumbled again over this issue, but this time there's a patch, D7 & D8.
This is very important for styles cascading, without it they are totally broken and you must resort to increased specificity (bad).

Comment #2

1 November 2012 at 12:47

Status:

Needs review

» Needs work

The last submitted patch, D7-rtl-overrides-weights-1372170.patch, failed testing.

Comment #3

tsi commented 1 November 2012 at 13:04

Status:

Needs work

» Needs review

#1: rtl-overrides-weights-1372170.patch queued for re-testing.

Comment #4

tsi commented 1 November 2012 at 13:10

#1: D7-rtl-overrides-weights-1372170.patch queued for re-testing.

[EDIT] - nevermind this one, it is being tested against D8 :(

Comment #5

1 November 2012 at 13:14

Status:

Needs review

» Needs work

The last submitted patch, D7-rtl-overrides-weights-1372170.patch, failed testing.

Comment #6

yakoub commented 1 November 2012 at 13:52

changing wight increment won't change the order because parent.css and child.css have weight 0 and group CSS_THEME so when hook_css_alter runs then parent-rtl.css and child-rtl.css will get same weight of 0+0.1 or 0+000.1 doesn't matter how small the incr

what should happen in drupal_add_css is adding by default a weight that is incremented by 1 and then for example parent.css will get weight = 10 and child.css weight=11 and in hook_css_alter parent-rtl weight will be 10.01 and child-rtl.css 11.01
but that is difficult to implement and not worth the trouble :) .. just use child-rtl.css for your overrides and not child.css

Comment #7

tsi commented 1 November 2012 at 14:09

@yakoub, we already do.

    // Always add a tiny value to the weight, to conserve the insertion order.
    $options['weight'] += count($css) / 1000;

http://api.drupal.org/api/drupal/core%21includes%21common.inc/function/d...

Comment #8

yakoub commented 1 November 2012 at 15:10

yes i missed it :)

Comment #9

tsi commented 6 November 2012 at 10:10

So do you think it is RTBC ? I can't review my own patch...

Comment #10

tsi commented 21 November 2012 at 09:27

Status:

Needs work

» Reviewed & tested by the community

I would mark this "needs review" but the bot will fail over the D7 patch. anyway, no work to be done here...

Comment #11

sun

German

Karlsruhe

commented 25 November 2012 at 22:07

Looks good.

An additional inline comment to explain the very small value plus a reference to essentially what #7 referred to would have been great to prevent this mistake from happening in the future, but I guess it's also fine without.