From 9a0f130018f0334aa9883a378287c3e374fccb13 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 31 Jul 2026 16:37:13 +0200 Subject: [PATCH 1/2] Support declaring font faces in theme options Loading custom fonts required referencing a stylesheet pack, which means a code deploy per font and an extra stylesheet request before the browser can start downloading font files. Font faces can now be declared in theme options instead, pointing either at files in the theme's asset directory or at uploaded theme customization files. The latter prepares letting admins upload fonts in site settings. Loading fonts via stylesheet packs keeps working unchanged. REDMINE-21330 --- .../pageflow_scrolled/themes_helper.rb | 124 +++++++ .../doc/creating_themes/custom_typography.md | 118 ++++++- entry_types/scrolled/lib/pageflow_scrolled.rb | 6 + entry_types/scrolled/spec/fixtures/font.woff | Bin 0 -> 44 bytes entry_types/scrolled/spec/fixtures/font.woff2 | Bin 0 -> 48 bytes .../pageflow_scrolled/themes_helper_spec.rb | 320 ++++++++++++++++++ .../scrolled/spec/pageflow_scrolled_spec.rb | 56 +++ 7 files changed, 615 insertions(+), 9 deletions(-) create mode 100644 entry_types/scrolled/spec/fixtures/font.woff create mode 100644 entry_types/scrolled/spec/fixtures/font.woff2 diff --git a/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb b/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb index 3c4cd943f7..0ec87fd906 100644 --- a/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb +++ b/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb @@ -32,6 +32,7 @@ def scrolled_theme_properties_style_tag(theme) ].flatten content_tag('style', raw(<<~CSS), data: {theme: ''}) + #{scrolled_theme_font_face_rules(theme)} :root { #{declarations.join("\n")} } @@ -41,6 +42,12 @@ def scrolled_theme_properties_style_tag(theme) CSS end + def scrolled_theme_font_face_rules(theme) + theme.options.fetch(:font_faces, []).filter_map { |face| + FontFaceRule.new(face, theme:) { |path| scrolled_theme_asset_path(theme, path) }.generate + }.join("\n") + end + def scrolled_theme_typography_rules(theme) RuleSet.new(prefix: 'typography').generate(theme.options.fetch(:typography, {})) end @@ -52,6 +59,123 @@ def scrolled_theme_properties_rules(theme) private + # @api private + class FontFaceRule + FORMATS = { + '.woff2' => 'woff2', + '.woff' => 'woff', + '.ttf' => 'truetype', + '.otf' => 'opentype' + }.freeze + + WEIGHT_PATTERN = /\A(normal|bold|\d{1,4}( \d{1,4})?)\z/ + STYLE_PATTERN = /\A(normal|italic)\z/ + FORMAT_PATTERN = /\A(woff2?|truetype|opentype|embedded-opentype|svg)(-variations)?\z/ + UNICODE_RANGE_PATTERN = /\A\s*U\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})? + (\s*,\s*U\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})?)*\s*\z/xi + + # Quotes and backslashes would allow breaking out of the quoted + # strings family names and urls are interpolated into. + UNSAFE_IN_FAMILY = /["'\\[[:cntrl:]]]/ + UNSAFE_IN_URL = /["'\\\s]/ + + ABSOLUTE_URL = %r{\A((https?:)?//|/|data:)} + + def initialize(face, theme:, &resolve_path) + @face = face + @theme = theme + @resolve_path = resolve_path + end + + def generate + return if family.blank? || source_values.empty? + + <<~CSS + @font-face { + #{declarations.join("\n ")} + } + CSS + end + + private + + attr_reader :face, :theme + + def declarations + [ + %(font-family: "#{family}";), + "src: #{source_values.join(', ')};", + 'font-display: swap;', + *descriptors + ] + end + + def family + @family ||= face[:family].to_s.gsub(UNSAFE_IN_FAMILY, '').strip + end + + def source_values + @source_values ||= + sources + .reject { |source| source[:url].match?(UNSAFE_IN_URL) } + .map { |source| source_value(source[:url], source[:format]) } + end + + def sources + if face[:file_role] + sources_from_files + else + sources_from_src + end + end + + def sources_from_files + Array(face[:file_role]).compact.filter_map do |role| + url = theme.files.dig(role.to_sym, :original) + {url: url.to_s, format: face[:format]} if url + end + end + + def sources_from_src + Array(face[:src]).filter_map do |src| + src = {url: src} unless src.is_a?(Hash) + next if src[:url].blank? + + {url: resolve_url(src[:url]), format: src.fetch(:format, face[:format])} + end + end + + def resolve_url(url) + return url.to_s if url.to_s.match?(ABSOLUTE_URL) + + @resolve_path.call(url).to_s + end + + def source_value(url, format) + format = [format, FORMATS[extension(url)]].find do |candidate| + candidate.to_s.match?(FORMAT_PATTERN) + end + + return %(url("#{url}")) unless format + + %(url("#{url}") format("#{format}")) + end + + def extension(url) + File.extname(url.split(/[?#]/).first.to_s).downcase + end + + def descriptors + [ + ['font-weight', face[:weight], WEIGHT_PATTERN], + ['font-style', face[:style], STYLE_PATTERN], + ['unicode-range', face[:unicode_range], UNICODE_RANGE_PATTERN] + ].filter_map do |property, value, pattern| + "#{property}: #{value.to_s.strip};" if value.to_s.match?(pattern) + end + end + end + BREAKPOINTS = { sm: '640px', md: '768px', diff --git a/entry_types/scrolled/doc/creating_themes/custom_typography.md b/entry_types/scrolled/doc/creating_themes/custom_typography.md index 4c80cbbf37..c62ac86643 100644 --- a/entry_types/scrolled/doc/creating_themes/custom_typography.md +++ b/entry_types/scrolled/doc/creating_themes/custom_typography.md @@ -2,9 +2,113 @@ ## Custom Fonts -[Fontsource](https://github.com/fontsource/fontsource) is the -recommended way to load custom fonts. Add the npm package for the -font: +Place the font files inside the theme's asset directory and declare +one font face per weight/style combination: + +``` ruby +entry_type_config.themes.register(:my_custom_theme, + # ... + font_faces: [ + {family: 'Open Sans', + weight: '400', + style: 'normal', + src: 'fonts/open-sans-400-normal.woff2'}, + {family: 'Open Sans', + weight: '700', + style: 'normal', + src: 'fonts/open-sans-700-normal.woff2'} + ], + properties: { + root: { + entry_font_family: '"Open Sans", sans-serif', + widget_font_family: '"Open Sans", sans-serif' + } + }) +``` + +Different fonts can be used for the main content of the entry and +widgets. + +The resulting `@font-face` rules are rendered into the same style tag +as the theme's other custom properties - both in published entries and +in the editor. Since no separate stylesheet needs to be requested, the +browser can start loading font files earlier. + +The following keys are supported: + +| Key | Description | +| --- | ----------- | +| `family` | Font family name to reference in font family properties and typography rules. | +| `src` | Path or url of the font file. See below. | +| `format` | Format of the font file. Derived from the file extension by default. | +| `weight` | Weight provided by the font file. Either a single value or a range like `'300 900'` for variable fonts. | +| `style` | Either `normal` or `italic`. | +| `unicode_range` | Code points provided by the font file. See below. | +| `file_role` | Role of an uploaded theme customization file to use instead of `src`. | + +`font-display: swap` is always included so that text remains visible +while font files are loading. Faces with invalid values are skipped. + +### Font File Paths + +Relative `src` paths are resolved inside the theme's asset directory, +just like icons and logos. Paths starting with `../shared/` refer to +the shared theme directory: + +``` ruby +src: '../shared/fonts/open-sans-400-normal.woff2' +``` + +Absolute urls and paths are used as is. + +### Multiple Formats + +Pass an array to let the browser pick the first format it supports: + +``` ruby +{family: 'Open Sans', + weight: '400', + src: ['fonts/open-sans-400-normal.woff2', + 'fonts/open-sans-400-normal.woff']} +``` + +The `format` key applies to all sources of the face. Use hashes to +specify formats per source: + +``` ruby +{family: 'Open Sans Variable', + weight: '300 900', + src: [{url: 'fonts/open-sans-wght-normal.woff2', format: 'woff2-variations'}, + 'fonts/open-sans-400-normal.woff']} +``` + +### Reducing Font File Size + +Fonts that support many scripts can be split into subsets. Declare one +face per subset and use `unicode_range` to let the browser download +only those subsets that contain code points used in the entry: + +``` ruby +font_faces: [ + {family: 'Open Sans', + weight: '400', + src: 'fonts/open-sans-latin-400-normal.woff2', + unicode_range: 'U+0000-00FF, U+0131, U+0152-0153'}, + {family: 'Open Sans', + weight: '400', + src: 'fonts/open-sans-latin-ext-400-normal.woff2', + unicode_range: 'U+0100-024F, U+0259, U+1E00-1EFF'} +] +``` + +Packages published by +[Fontsource](https://github.com/fontsource/fontsource) contain the +ranges of their subsets in a `unicode.json` file. + +### Loading Fonts via Stylesheet Packs + +Alternatively, fonts can be loaded by referencing a stylesheet pack +which contains `@font-face` rules. Add the npm package for the font: $ yarn add @fontsource/open-sans @@ -16,13 +120,12 @@ Create a Webpacker entry point file for your font: @import "@fontsource/open-sans/700.css"; ``` -Adjust theme options to load the font stylesheet pack and set the font -family properties: +Adjust theme options to load the font stylesheet pack: ``` ruby entry_type_config.themes.register(:my_custom_theme, # ... - stylesheet_packs: ['font/openSans'], + stylesheet_packs: ['fonts/openSans'], properties: { root: { entry_font_family: '"Open Sans", sans-serif', @@ -31,9 +134,6 @@ entry_type_config.themes.register(:my_custom_theme, }) ``` -Different fonts can be used for the main content of the entry and -widgets. - ## Typography Rules Aspects like font size, font weight, letter spacing, margins etc. can diff --git a/entry_types/scrolled/lib/pageflow_scrolled.rb b/entry_types/scrolled/lib/pageflow_scrolled.rb index f119e13df4..86a844171b 100644 --- a/entry_types/scrolled/lib/pageflow_scrolled.rb +++ b/entry_types/scrolled/lib/pageflow_scrolled.rb @@ -17,6 +17,7 @@ def entry_type theme_files: { logo_mobile: LOGO_OPTIONS, logo_desktop: LOGO_OPTIONS, + font: FONT_OPTIONS, **FAVICONS }) end @@ -40,6 +41,11 @@ def editor_fragment_renderer end }.freeze + FONT_OPTIONS = { + content_type: %r{\A(font/woff2?|application/(x-)?font-woff2?)\z}, + styles: {original: {}} + }.freeze + FAVICONS = { favicon: { content_type: %r{\Aimage/svg\+xml\z}, diff --git a/entry_types/scrolled/spec/fixtures/font.woff b/entry_types/scrolled/spec/fixtures/font.woff new file mode 100644 index 0000000000000000000000000000000000000000..fd42de3182bcf2e6e98df7bace664d605491f56c GIT binary patch literal 44 ZcmXT-cXMN4WB>sjFbN_cd^i`z002IL0f7Jj literal 0 HcmV?d00001 diff --git a/entry_types/scrolled/spec/fixtures/font.woff2 b/entry_types/scrolled/spec/fixtures/font.woff2 new file mode 100644 index 0000000000000000000000000000000000000000..dd9ee6f23ebfeeaf788154be6cb3ab39bf22c847 GIT binary patch literal 48 ZcmXT-cQayOWB>sJFbN|dVlW<@1^_{k0dW8T literal 0 HcmV?d00001 diff --git a/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb b/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb index 777fdb8985..e3950b3f2e 100644 --- a/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb +++ b/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb @@ -96,6 +96,19 @@ module PageflowScrolled visible: false) end + it 'renders style tag with font face rules for theme' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: 'https://cdn.example.com/a.woff2'} + ]) + + html = helper.scrolled_theme_properties_style_tag(theme) + + expect(html).to have_css('style[data-theme]', + text: /@font-face.*font-family: "Avenir";/m, + visible: false) + end + it 'renders style tag with custom typography rules for theme' do theme = Pageflow::Theme.new(:test, typography: { @@ -112,6 +125,313 @@ module PageflowScrolled end end + describe '#scrolled_theme_font_face_rules' do + before do + allow(helper).to receive(:asset_pack_path) { |path| "/packs/#{path}" } + end + + def customized_theme(options) + Pageflow::CustomizedTheme.find(entry: create(:entry), + theme: Pageflow::Theme.new(:test, options)) + end + + def upload_font(entry, file_name) + Pageflow.theme_customizations.upload_file( + site: entry.site, + entry_type_name: 'scrolled', + type_name: :font, + attachment: fixture_file_upload(file_name) + ) + end + + it 'renders rule for each font face of theme' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', + weight: '400', + style: 'italic', + src: '/fonts/a.woff'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include(<<~CSS) + @font-face { + font-family: "Avenir"; + src: url("/fonts/a.woff") format("woff"); + font-display: swap; + font-weight: 400; + font-style: italic; + } + CSS + end + + it 'resolves relative src in theme directory' do + theme = customized_theme(font_faces: [{family: 'Avenir', src: 'fonts/a.woff'}]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('url("/packs/static/pageflow-scrolled/themes/test/fonts/a.woff")') + end + + it 'resolves relative src in shared theme directory' do + theme = customized_theme(font_faces: [{family: 'Avenir', src: '../shared/fonts/a.woff'}]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('url("/packs/static/pageflow-scrolled/themes/shared/fonts/a.woff")') + end + + it 'passes absolute urls through' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: 'https://cdn.example.com/a.woff2'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('url("https://cdn.example.com/a.woff2") format("woff2")') + end + + it 'renders multiple sources with formats derived from extension' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: ['/fonts/a.woff2', '/fonts/a.woff']} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('src: url("/fonts/a.woff2") format("woff2"), ' \ + 'url("/fonts/a.woff") format("woff");') + end + + it 'allows overriding format of all sources' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Figtree Variable', + src: '/fonts/a.woff2', + format: 'woff2-variations'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('url("/fonts/a.woff2") format("woff2-variations")') + end + + it 'allows specifying format per source' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', + src: [{url: '/fonts/a.woff2', format: 'woff2-variations'}, + '/fonts/a.woff']} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('src: url("/fonts/a.woff2") format("woff2-variations"), ' \ + 'url("/fonts/a.woff") format("woff");') + end + + it 'omits format for unknown extension' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: '/fonts/a.bin'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('src: url("/fonts/a.bin");') + end + + it 'renders unicode range' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', + src: '/fonts/a.woff', + unicode_range: 'U+0000-00FF, U+0131, U+2C60-2C7F'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('unicode-range: U+0000-00FF, U+0131, U+2C60-2C7F;') + end + + it 'renders variable font weight range' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: '/fonts/a.woff2', weight: '300 900'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('font-weight: 300 900;') + end + + it 'renders numeric weight' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: '/fonts/a.woff', weight: 400} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('font-weight: 400;') + end + + it 'skips invalid weight, style, format and unicode range' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', + src: '/fonts/a.woff', + weight: 'bold; } body {display: none', + style: 'italic;}', + format: 'woff") format("collection', + unicode_range: 'U+00; } body {display: none'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('src: url("/fonts/a.woff") format("woff");') + expect(css).not_to include('display: none') + expect(css).not_to include('font-weight') + expect(css).not_to include('font-style') + expect(css).not_to include('unicode-range') + end + + it 'strips quotes and backslashes from family' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Ave"n\\ir', src: '/fonts/a.woff'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to include('font-family: "Avenir";') + end + + it 'keeps family from breaking out of quoted string' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'X"; } .injected {color: red', + src: '/fonts/a.woff'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css[/font-family: .*/]).to eq('font-family: "X; } .injected {color: red";') + end + + it 'skips sources with quotes or whitespace in url' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {family: 'Avenir', src: '/fonts/a".woff'}, + {family: 'Oswald', src: '/fonts/a b.woff'} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to be_blank + end + + it 'skips face without family or sources' do + theme = Pageflow::Theme.new(:test, + font_faces: [ + {src: '/fonts/a.woff'}, + {family: 'Avenir'}, + {family: 'Oswald', src: []}, + {family: 'Karla', src: [{format: 'woff2'}]}, + {family: 'Lato', file_role: [nil]} + ]) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to be_blank + end + + it 'handles missing theme option' do + theme = Pageflow::Theme.new(:test) + + css = helper.scrolled_theme_font_face_rules(theme) + + expect(css).to be_blank + end + + it 'resolves file role of uploaded theme customization file' do + entry = create(:published_entry, type_name: 'scrolled') + file = upload_font(entry, 'font.woff2') + Pageflow.theme_customizations.update( + site: entry.site, + entry_type_name: 'scrolled', + overrides: { + font_faces: [{family: 'font1', weight: '400', file_role: 'font_font1_400_normal'}] + }, + file_ids: {font_font1_400_normal: file.id} + ) + + css = helper.scrolled_theme_font_face_rules(entry.theme) + + expect(css).to include('font-family: "font1";') + expect(css).to match(%r{src: url\("[^"]*original/font\.woff2[^"]*"\) format\("woff2"\);}) + end + + it 'resolves multiple file roles of one face' do + entry = create(:published_entry, type_name: 'scrolled') + woff2 = upload_font(entry, 'font.woff2') + woff = upload_font(entry, 'font.woff') + Pageflow.theme_customizations.update( + site: entry.site, + entry_type_name: 'scrolled', + overrides: { + font_faces: [{family: 'font1', + file_role: ['font_font1_400_normal_woff2', + 'font_font1_400_normal_woff']}] + }, + file_ids: {font_font1_400_normal_woff2: woff2.id, + font_font1_400_normal_woff: woff.id} + ) + + css = helper.scrolled_theme_font_face_rules(entry.theme) + + expect(css).to match(/format\("woff2"\), url\("[^"]*\.woff[^"]*"\) format\("woff"\);/) + end + + it 'applies format to file role sources' do + entry = create(:published_entry, type_name: 'scrolled') + file = upload_font(entry, 'font.woff2') + Pageflow.theme_customizations.update( + site: entry.site, + entry_type_name: 'scrolled', + overrides: { + font_faces: [{family: 'font1', + weight: '300 900', + file_role: 'font_font1_wght_normal', + format: 'woff2-variations'}] + }, + file_ids: {font_font1_wght_normal: file.id} + ) + + css = helper.scrolled_theme_font_face_rules(entry.theme) + + expect(css).to include('format("woff2-variations");') + expect(css).to include('font-weight: 300 900;') + end + + it 'skips face with unresolvable file role' do + entry = create(:published_entry, type_name: 'scrolled') + Pageflow.theme_customizations.update( + site: entry.site, + entry_type_name: 'scrolled', + overrides: { + font_faces: [{family: 'font1', file_role: 'font_font1_400_normal'}] + } + ) + + css = helper.scrolled_theme_font_face_rules(entry.theme) + + expect(css).to be_blank + end + end + describe '#scrolled_theme_typography_rules' do it 'returns rules for theme' do theme = Pageflow::Theme.new(:test, diff --git a/entry_types/scrolled/spec/pageflow_scrolled_spec.rb b/entry_types/scrolled/spec/pageflow_scrolled_spec.rb index 9b0469f2b7..6834c024a1 100644 --- a/entry_types/scrolled/spec/pageflow_scrolled_spec.rb +++ b/entry_types/scrolled/spec/pageflow_scrolled_spec.rb @@ -4,4 +4,60 @@ it 'has an engine' do expect(PageflowScrolled::Engine).not_to be nil end + + describe 'font theme file type' do + it 'accepts woff upload', unstub_paperclip: true do + entry = create(:published_entry, type_name: 'scrolled') + + file = Pageflow.theme_customizations.upload_file( + site: entry.site, + entry_type_name: 'scrolled', + type_name: :font, + attachment: fixture_file_upload('font.woff') + ) + + expect(file.attachment_content_type).to eq('font/woff') + expect(file.urls[:original]).to match(%r{original/font\.woff}) + end + + it 'accepts woff2 upload', unstub_paperclip: true do + entry = create(:published_entry, type_name: 'scrolled') + + file = Pageflow.theme_customizations.upload_file( + site: entry.site, + entry_type_name: 'scrolled', + type_name: :font, + attachment: fixture_file_upload('font.woff2') + ) + + expect(file.attachment_content_type).to eq('font/woff2') + expect(file.urls[:original]).to match(%r{original/font\.woff2}) + end + + it 'does not process uploaded fonts' do + entry = create(:published_entry, type_name: 'scrolled') + + file = Pageflow.theme_customizations.upload_file( + site: entry.site, + entry_type_name: 'scrolled', + type_name: :font, + attachment: fixture_file_upload('font.woff2') + ) + + expect(file.attachment_styles).to be_empty + end + + it 'rejects uploads that are not fonts', unstub_paperclip: true do + entry = create(:published_entry, type_name: 'scrolled') + + expect { + Pageflow.theme_customizations.upload_file( + site: entry.site, + entry_type_name: 'scrolled', + type_name: :font, + attachment: fixture_file_upload('image.svg') + ) + }.to raise_error(ActiveRecord::RecordInvalid) + end + end end From d5761b12f45f05125b2dd71b7ccd26ce1984973a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 31 Jul 2026 17:54:56 +0200 Subject: [PATCH 2/2] Avoid mutating paths passed for theme assets Resolving a path in the shared theme directory stripped the prefix from the string it was given. Callers passing the same string object more than once - like theme options declaring font faces - got the wrong directory on every render after the first. --- .../pageflow_scrolled/themes_helper.rb | 2 +- .../pageflow_scrolled/themes_helper_spec.rb | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb b/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb index 0ec87fd906..01bb6eea36 100644 --- a/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb +++ b/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb @@ -252,7 +252,7 @@ def scrolled_theme_deep_declarations(hash, suffix = nil, prefix = []) def extract_theme_directory_from_scrolled_theme_asset_path(theme, path) if path.starts_with?('../shared/') - ['shared', path.gsub!('../shared/', '')] + ['shared', path.sub('../shared/', '')] elsif path.starts_with?('../') raise(ArgumentError, 'Upward navigation to other directory than the shared ' \ diff --git a/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb b/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb index e3950b3f2e..ada62bb3b4 100644 --- a/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb +++ b/entry_types/scrolled/spec/helpers/pageflow_scrolled/themes_helper_spec.rb @@ -27,6 +27,18 @@ module PageflowScrolled helper.scrolled_theme_asset_path(customized_theme, '../shared/icons/muted.svg') end + it 'does not modify passed path' do + entry = create(:entry) + theme = Pageflow::Theme.new(:test) + customized_theme = Pageflow::CustomizedTheme.find(entry:, theme:) + path = '../shared/icons/muted.svg' + + allow(helper).to receive(:asset_pack_path) + helper.scrolled_theme_asset_path(customized_theme, path) + + expect(path).to eq('../shared/icons/muted.svg') + end + it 'raises helpful error for relative paths to other sibling or parent directory' do entry = create(:entry) theme = Pageflow::Theme.new(:test) @@ -182,6 +194,14 @@ def upload_font(entry, file_name) expect(css).to include('url("/packs/static/pageflow-scrolled/themes/shared/fonts/a.woff")') end + it 'resolves src in shared theme directory on repeated renders' do + theme = customized_theme(font_faces: [{family: 'Avenir', src: '../shared/fonts/a.woff'}]) + + first = helper.scrolled_theme_font_face_rules(theme) + + expect(helper.scrolled_theme_font_face_rules(theme)).to eq(first) + end + it 'passes absolute urls through' do theme = Pageflow::Theme.new(:test, font_faces: [