FIX: Refactor to prevent themes affecting core stylesheets (#7029)

FIX: Refactor to prevent themes affecting core stylesheets (#7029)

If a theme setting contained invalid SCSS, it would cause an error 500 on the site, with no way to recover. This commit stops loading theme settings in the core stylesheets, and instead only loads the color scheme variables. This change also makes common/foundation/variables.scss available to themes without an explicit import.

diff --git a/app/assets/stylesheets/common/foundation/variables.scss b/app/assets/stylesheets/common/foundation/variables.scss
index 29e1ebb..41afa8d 100644
--- a/app/assets/stylesheets/common/foundation/variables.scss
+++ b/app/assets/stylesheets/common/foundation/variables.scss
@@ -64,7 +64,7 @@ $line-height-large: 1.4; // Normal or small text
 // These files don't actually exist. They're injected by Stylesheet::Compiler.
 // --------------------------------------------------
 
-@import "theme_variables";
+@import "theme_colors";
 @import "plugins_variables";
 @import "common/foundation/math";
 
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index 4064cab..9d62dc2 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -261,7 +261,7 @@ class ThemeField < ActiveRecord::Base
   def ensure_scss_compiles!
     if ThemeField.scss_fields.include?(self.name)
       begin
-        Stylesheet::Compiler.compile("@import \"theme_variables\"; @import \"theme_field\";",
+        Stylesheet::Compiler.compile("@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"theme_field\";",
                                      "theme.scss",
                                      theme_field: self.value.dup,
                                      theme: self.theme
diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb
index 9f7b3b9..1c2a165 100644
--- a/lib/stylesheet/compiler.rb
+++ b/lib/stylesheet/compiler.rb
@@ -10,7 +10,7 @@ module Stylesheet
 
       if Importer.special_imports[asset.to_s]
         filename = "theme.scss"
-        file = "@import \"theme_variables\"; @import \"#{asset}\";"
+        file = "@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"#{asset}\";"
       else
         filename = "#{asset}.scss"
         path = "#{ASSET_ROOT}/#{filename}"
diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb
index 635645c..49ff26d 100644
--- a/lib/stylesheet/importer.rb
+++ b/lib/stylesheet/importer.rb
@@ -33,12 +33,19 @@ module Stylesheet
       import_files(DiscoursePluginRegistry.sass_variables)
     end
 
-    register_import "theme_variables" do
+    register_import "theme_colors" do
       contents = ""
       colors = (@theme_id && theme.color_scheme) ? theme.color_scheme.resolved_colors : ColorScheme.base_colors
       colors.each do |n, hex|
         contents << "$#{n}: ##{hex} !default;\n"
       end
+      contents
+
+      Import.new("theme_colors.scss", source: contents)
+    end
+
+    register_import "theme_variables" do
+      contents = ""
 
       theme&.all_theme_variables&.each do |field|
         if field.type_id == ThemeField.types[:theme_upload_var]
diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb
index a799ac0..0b4a332 100644
--- a/spec/models/theme_spec.rb
+++ b/spec/models/theme_spec.rb
@@ -290,7 +290,7 @@ HTML
       theme.reload
       expect(theme.theme_fields.find_by(name: :scss).error).to eq(nil)
 
-      scss, _map = Stylesheet::Compiler.compile('@import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
+      scss, _map = Stylesheet::Compiler.compile('@import "common/foundation/variables"; @import "theme_variables"; @import "desktop_theme"; ', "theme.scss", theme_id: theme.id)
       expect(scss).to include(upload.url)
     end
   end

GitHub sha: 7878e500

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there: