FIX: Replace R2 gem with rtlcss for generating RTL CSS (#19636)

We've had a couple of problems with the R2 gem where it generated a broken RTL CSS bundle that caused a badly broken layout when Discourse is used in an RTL language, see a3ce93b and 5926386. For this reason, we're replacing R2 with `rtlcss` that can handle modern CSS features better than R2 does.

`rltcss` is written in JS and available as an npm package. Calling the `rltcss` from rubyland is done via the `rtlcss_wrapper` gem which contains a distributable copy of the `rtlcss` package and loads/calls it with Mini Racer. See https://github.com/discourse/rtlcss_wrapper for more details.

Internal topic: t/76263.
This commit is contained in:
Osama Sayegh 2023-02-01 14:21:15 +03:00 committed by GitHub
parent 66256c15bd
commit f94951147e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 15 additions and 27 deletions

View File

@ -105,7 +105,7 @@ gem "pg"
gem "mini_sql"
gem "pry-rails", require: false
gem "pry-byebug", require: false
gem "r2", require: false
gem "rtlcss_wrapper", require: false
gem "rake"
gem "thor", require: false

View File

@ -315,7 +315,6 @@ GEM
public_suffix (5.0.1)
puma (6.0.2)
nio4r (~> 2.0)
r2 (0.2.7)
racc (1.6.2)
rack (2.2.6.2)
rack-mini-profiler (3.0.0)
@ -398,6 +397,8 @@ GEM
json-schema (>= 2.2, < 4.0)
railties (>= 3.1, < 7.1)
rspec-core (>= 2.14)
rtlcss_wrapper (0.1.0)
mini_racer (~> 0.6.3)
rubocop (1.44.1)
json (~> 2.3)
parallel (~> 1.10)
@ -600,7 +601,6 @@ DEPENDENCIES
pry-byebug
pry-rails
puma
r2
rack
rack-mini-profiler
rack-protection
@ -621,6 +621,7 @@ DEPENDENCIES
rspec-rails
rss
rswag-specs
rtlcss_wrapper
rubocop-discourse
ruby-prof
ruby-readability

View File

@ -44,27 +44,7 @@
margin: 0 calc(var(--d-sidebar-row-horizontal-padding) * 2 / 3);
.sidebar-row {
// the multiplication by 1 here is a workaround for a bug in the R2 gem
// that we use to generate RTL CSS.
// the gem generates RTL CSS by converting anything left to right and
// vice versa. for example, a `padding-right: 1px;` property becomes
// `padding-left: 1px;` when it goes through the gem.
// the gem also handles the `padding` property (and similar properties)
// when it's in the 4-sides form, e.g. `padding: 1px 2px 3px 4px;` which
// gets converted to `padding: 1px 4px 3px 2px;`.
// however, the problem is that the gem detects 4-sides properties pretty
// naively - it splits the property value on /\s+/ and if it has 4 parts,
// it swaps the second and fourth parts.
// if you remove the by 1 multiplication in our rule below, we end up
// with a value that can be split into 4 parts and that causes the R2 gem
// to convert the rule to this:
// padding: 0.33rem 3) / calc(var(--d-sidebar-row-horizontal-padding);
// which is clearly invalid and breaks all the rules that come after this
// one in the application CSS bundle.
// in the long term we should probably find (or write ourselves)
// something that's smarter than R2, but for now let's workaround the bug
// in R2.
padding: 0.33rem calc(1 * var(--d-sidebar-row-horizontal-padding) / 3);
padding: 0.33rem calc(var(--d-sidebar-row-horizontal-padding) / 3);
}
}

View File

@ -55,7 +55,7 @@ a.hashtag-cooked {
background: linear-gradient(
to bottom,
rgba(var(--secondary-rgb), 0),
rgba(var(--secondary-rgb), 1)
rgba(var(--secondary-rgb), 100%)
);
}
}

View File

@ -65,8 +65,8 @@ module Stylesheet
result = engine.render
if options[:rtl]
require "r2"
[R2.r2(result), nil]
require "rtlcss_wrapper"
[RtlcssWrapper.flip_css(result), nil]
else
source_map = engine.source_map
source_map.force_encoding("UTF-8")

View File

@ -214,4 +214,11 @@ RSpec.describe Stylesheet::Compiler do
expect(refs).to eq([])
end
end
describe ".compile" do
it "produces RTL CSS when rtl option is given" do
css, _ = Stylesheet::Compiler.compile("a{right:1px}", "test.scss", rtl: true)
expect(css).to eq("a{left:1px}")
end
end
end