From e7df7a54b3c3136df1cc66eaa22fa99ef0bcb09a Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Wed, 5 Mar 2025 13:44:51 -0600 Subject: [PATCH 1/9] Use full-text link style for GLFM markdown headings Use the full text of the header in the anchor element for GLFM markdown headings, and update SCSS files to use a simple link icon decorator. Changelog: changed --- Gemfile | 2 +- Gemfile.checksum | 14 +++--- Gemfile.lock | 4 +- Gemfile.next.checksum | 14 +++--- Gemfile.next.lock | 4 +- .../stylesheets/framework/markdown_area.scss | 8 ---- .../stylesheets/framework/typography.scss | 41 +++++------------ app/assets/stylesheets/pages/issues.scss | 22 --------- .../banzai_pipeline_and_parsing.md | 2 +- lib/banzai/filter/include_filter.rb | 2 +- .../filter/markdown_engines/glfm_markdown.rb | 7 +-- lib/banzai/filter/markdown_filter.rb | 2 +- .../markdown_engines/glfm_markdown_spec.rb | 46 +++++++++---------- 13 files changed, 60 insertions(+), 108 deletions(-) diff --git a/Gemfile b/Gemfile index f24d2528913849..29eb0df5195856 100644 --- a/Gemfile +++ b/Gemfile @@ -274,7 +274,7 @@ gem 'asciidoctor-kroki', '~> 0.10.0', require: false, feature_category: :markdow gem 'rouge', '~> 4.6.1', feature_category: :shared gem 'truncato', '~> 0.7.13', feature_category: :team_planning gem 'nokogiri', '~> 1.18', feature_category: :shared -gem 'gitlab-glfm-markdown', '~> 0.0.38', feature_category: :markdown +gem 'gitlab-glfm-markdown', '~> 0.0.39', feature_category: :markdown gem 'tanuki_emoji', '~> 0.13', feature_category: :markdown gem 'unicode-emoji', '~> 4.0', feature_category: :markdown diff --git a/Gemfile.checksum b/Gemfile.checksum index 2efcd20c363c9f..736f7bf5cc3e1f 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -227,13 +227,13 @@ {"name":"gitlab-dangerfiles","version":"4.10.0","platform":"ruby","checksum":"0adb9cfec58ffce42f68b1aef528503bdc89aed3994ba461c67e1d9246513e1c"}, {"name":"gitlab-experiment","version":"1.0.0","platform":"ruby","checksum":"9ff76fe55d30012f0531be97143f3af5f74dd1ef3f33d630f320de0ceec9eab9"}, {"name":"gitlab-fog-azure-rm","version":"2.4.0","platform":"ruby","checksum":"678b86e542a37eda10e63ca02d04c9ff998b771df4aabc1f87e5c20148cb360b"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"aarch64-linux-gnu","checksum":"cc07d942d61c2efab764aa5065e21dc336128d8562db7e0815d0a060689f50fb"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"aarch64-linux-musl","checksum":"4efb65768d641a920583c885d26f255f180e4fe4b3016c86e58e2dfa099fe480"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"arm64-darwin","checksum":"4599b09016698b38bf407fe99139893175bc5d83ccc9040bd77faeceaabdc96f"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"ruby","checksum":"8e57dd9283d655a40f6120dac0728f1d088900228977c139e3c969c756f40bc6"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"x86_64-darwin","checksum":"0be7832fd9066e296abfd6c465c98e920d0f5722844c75ba0e47867b5b767161"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"x86_64-linux-gnu","checksum":"6725a555a9d6b35e6a89570acb76b249e2248187d5a665c7581449a26fe2958f"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"x86_64-linux-musl","checksum":"34a741398ba503873d18dac70994284fefae3b3da3ce0fe6c4e97dd4ac002fc0"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"aarch64-linux-gnu","checksum":"3c589bf98fa04b3ad7760564af0892d53b7cfc7058153a0aab331cf15ffbf06f"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"aarch64-linux-musl","checksum":"c3f1ccd9f958b8be0dfeb4f3cb58a185afdc602318748509f009da58fdd49a51"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"arm64-darwin","checksum":"efddd0a07a1b26a1b9af61dc4d9246b9599877a1221a9e0ceb2a5e69ab664cc2"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"ruby","checksum":"a7d7d237203ebe290c2519b742c2c536ee94401a038ad63f043db9f4166fa0c7"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"x86_64-darwin","checksum":"9f326f6e6771e4fad240bf9bfb2acd85eb86eb44ef058c58d2ba81bc2e0075d5"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"x86_64-linux-gnu","checksum":"90a12179eb5a9d311a840dccbab7f6d61c52fef5e526586bcfc70154016af683"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"x86_64-linux-musl","checksum":"f353114bf503e3389159e22c8af1bd12fea7016081bf60ee05b269b80793c101"}, {"name":"gitlab-kas-grpc","version":"18.5.0.pre.rc4","platform":"ruby","checksum":"8efe8bc957572bad2ee90c22836b285eb65574591db5c8a7bc8080f69ddebcc6"}, {"name":"gitlab-labkit","version":"1.0.1","platform":"ruby","checksum":"6f354529ea9a898dc622bfd21ea4e0024dbab0d0d5a6496f0af7f3163013ecb2"}, {"name":"gitlab-license","version":"2.6.0","platform":"ruby","checksum":"2c1f8ae73835640ec77bf758c1d0c9730635043c01cf77902f7976e826d7d016"}, diff --git a/Gemfile.lock b/Gemfile.lock index 952927c84b9a57..c1584dc6cff752 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -771,7 +771,7 @@ GEM mime-types net-http-persistent (~> 4.0) nokogiri (~> 1, >= 1.10.8) - gitlab-glfm-markdown (0.0.38) + gitlab-glfm-markdown (0.0.39) rb_sys (~> 0.9.109) gitlab-kas-grpc (18.5.0.pre.rc4) grpc (~> 1.0) @@ -2192,7 +2192,7 @@ DEPENDENCIES gitlab-duo-workflow-service-client (~> 0.6)! gitlab-experiment (~> 1.0.0) gitlab-fog-azure-rm (~> 2.4.0) - gitlab-glfm-markdown (~> 0.0.38) + gitlab-glfm-markdown (~> 0.0.39) gitlab-grape-openapi! gitlab-housekeeper! gitlab-http! diff --git a/Gemfile.next.checksum b/Gemfile.next.checksum index 47dd47c16f3b16..b0eb3807c39ab7 100644 --- a/Gemfile.next.checksum +++ b/Gemfile.next.checksum @@ -227,13 +227,13 @@ {"name":"gitlab-dangerfiles","version":"4.10.0","platform":"ruby","checksum":"0adb9cfec58ffce42f68b1aef528503bdc89aed3994ba461c67e1d9246513e1c"}, {"name":"gitlab-experiment","version":"1.0.0","platform":"ruby","checksum":"9ff76fe55d30012f0531be97143f3af5f74dd1ef3f33d630f320de0ceec9eab9"}, {"name":"gitlab-fog-azure-rm","version":"2.4.0","platform":"ruby","checksum":"678b86e542a37eda10e63ca02d04c9ff998b771df4aabc1f87e5c20148cb360b"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"aarch64-linux-gnu","checksum":"cc07d942d61c2efab764aa5065e21dc336128d8562db7e0815d0a060689f50fb"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"aarch64-linux-musl","checksum":"4efb65768d641a920583c885d26f255f180e4fe4b3016c86e58e2dfa099fe480"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"arm64-darwin","checksum":"4599b09016698b38bf407fe99139893175bc5d83ccc9040bd77faeceaabdc96f"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"ruby","checksum":"8e57dd9283d655a40f6120dac0728f1d088900228977c139e3c969c756f40bc6"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"x86_64-darwin","checksum":"0be7832fd9066e296abfd6c465c98e920d0f5722844c75ba0e47867b5b767161"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"x86_64-linux-gnu","checksum":"6725a555a9d6b35e6a89570acb76b249e2248187d5a665c7581449a26fe2958f"}, -{"name":"gitlab-glfm-markdown","version":"0.0.38","platform":"x86_64-linux-musl","checksum":"34a741398ba503873d18dac70994284fefae3b3da3ce0fe6c4e97dd4ac002fc0"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"aarch64-linux-gnu","checksum":"3c589bf98fa04b3ad7760564af0892d53b7cfc7058153a0aab331cf15ffbf06f"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"aarch64-linux-musl","checksum":"c3f1ccd9f958b8be0dfeb4f3cb58a185afdc602318748509f009da58fdd49a51"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"arm64-darwin","checksum":"efddd0a07a1b26a1b9af61dc4d9246b9599877a1221a9e0ceb2a5e69ab664cc2"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"ruby","checksum":"a7d7d237203ebe290c2519b742c2c536ee94401a038ad63f043db9f4166fa0c7"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"x86_64-darwin","checksum":"9f326f6e6771e4fad240bf9bfb2acd85eb86eb44ef058c58d2ba81bc2e0075d5"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"x86_64-linux-gnu","checksum":"90a12179eb5a9d311a840dccbab7f6d61c52fef5e526586bcfc70154016af683"}, +{"name":"gitlab-glfm-markdown","version":"0.0.39","platform":"x86_64-linux-musl","checksum":"f353114bf503e3389159e22c8af1bd12fea7016081bf60ee05b269b80793c101"}, {"name":"gitlab-kas-grpc","version":"18.5.0.pre.rc4","platform":"ruby","checksum":"8efe8bc957572bad2ee90c22836b285eb65574591db5c8a7bc8080f69ddebcc6"}, {"name":"gitlab-labkit","version":"1.0.1","platform":"ruby","checksum":"6f354529ea9a898dc622bfd21ea4e0024dbab0d0d5a6496f0af7f3163013ecb2"}, {"name":"gitlab-license","version":"2.6.0","platform":"ruby","checksum":"2c1f8ae73835640ec77bf758c1d0c9730635043c01cf77902f7976e826d7d016"}, diff --git a/Gemfile.next.lock b/Gemfile.next.lock index 46437ce6e3121a..d19595c0f99171 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -765,7 +765,7 @@ GEM mime-types net-http-persistent (~> 4.0) nokogiri (~> 1, >= 1.10.8) - gitlab-glfm-markdown (0.0.38) + gitlab-glfm-markdown (0.0.39) rb_sys (~> 0.9.109) gitlab-kas-grpc (18.5.0.pre.rc4) grpc (~> 1.0) @@ -2187,7 +2187,7 @@ DEPENDENCIES gitlab-duo-workflow-service-client (~> 0.6)! gitlab-experiment (~> 1.0.0) gitlab-fog-azure-rm (~> 2.4.0) - gitlab-glfm-markdown (~> 0.0.38) + gitlab-glfm-markdown (~> 0.0.39) gitlab-grape-openapi! gitlab-housekeeper! gitlab-http! diff --git a/app/assets/stylesheets/framework/markdown_area.scss b/app/assets/stylesheets/framework/markdown_area.scss index b5351130b2bce2..3a89a3eca16837 100644 --- a/app/assets/stylesheets/framework/markdown_area.scss +++ b/app/assets/stylesheets/framework/markdown_area.scss @@ -65,14 +65,6 @@ min-height: 177px; padding: 10px 0; overflow-x: auto; - - a.anchor { - margin-left: -1rem !important; - } - - details a.anchor { - margin-left: -1.75rem !important; - } } .markdown-area { diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 6ccd218411de7b..b212416cf04790 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -83,16 +83,8 @@ @apply gl-mb-5; } - // Shift the anchor link for a heading inside a details summary to the left - // to make room for the summary toggle details summary { @apply gl-font-bold; - - h1, h2, h3, h4, h5, h6 { - a.anchor { - @apply -gl-ml-7; - } - } } summary > * { @@ -686,30 +678,21 @@ h4, h5, h6 { - $anchor-offset: 16px; - a.anchor { - margin-left: calc(-1 * $anchor-offset); - text-decoration: none; - outline: none; - position: absolute; - width: $anchor-offset; - - &::after { - @include gl-dark-invert-keep-hue; - content: url('icon_anchor.svg'); - visibility: hidden; + @apply gl-text-default gl-no-underline; + + &:hover, &:focus-visible { + &::after { + @apply gl-inline-block gl-bg-cover; + @include gl-dark-invert-keep-hue; + content: ""; + background-image: url('icon_anchor.svg'); + height: 0.9em; + width: 0.9em; + margin-inline-start: 0.2em; + } } } - - &:hover > a.anchor::after { - visibility: visible; - } - - > a.anchor:focus::after { - visibility: visible; - outline: auto; - } } .big { diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 966fcc0ce70563..d9779367c4d5d1 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -180,28 +180,6 @@ ul.related-merge-requests > li gl-emoji { } } -.issue-details { - .card-title { - a.anchor { - left: -16px; - top: 4px; - outline: none; - - &::after { - @include gl-dark-invert-keep-hue; - content: url('icon_anchor.svg'); - visibility: hidden; - } - } - - &:hover > a.anchor::after { - position: relative; - top: -3px; - visibility: visible; - } - } -} - .issue-sticky-header { left: var(--application-bar-left); right: var(--application-bar-right); diff --git a/doc/development/gitlab_flavored_markdown/banzai_pipeline_and_parsing.md b/doc/development/gitlab_flavored_markdown/banzai_pipeline_and_parsing.md index 692763de74957c..01176b4be76bc2 100644 --- a/doc/development/gitlab_flavored_markdown/banzai_pipeline_and_parsing.md +++ b/doc/development/gitlab_flavored_markdown/banzai_pipeline_and_parsing.md @@ -85,7 +85,7 @@ We use our [`gitlab-glfm-markdown`](https://gitlab.com/gitlab-org/ruby/gems/gitl `comrak` provides 100% compatibility with GFM and CommonMark while allowing additional extensions to be added to it. For example, we were able to implement our multi-line blockquote and wikilink syntax directly in `comrak`. The goal is to move more of the Ruby filters into either `comrak` (if it makes sense) or into `gitlab-glfm-markdown`. -For more information about the various options that get passed into `comrak`, see [glfm_markdown.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/banzai/filter/markdown_engines/glfm_markdown.rb#L12-L34). +For more information about the various options that get passed into `comrak`, see [glfm_markdown.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/banzai/filter/markdown_engines/glfm_markdown.rb#L12-L52). ## Caching diff --git a/lib/banzai/filter/include_filter.rb b/lib/banzai/filter/include_filter.rb index 8fdf55962d2cd4..f6400a5b5c1149 100644 --- a/lib/banzai/filter/include_filter.rb +++ b/lib/banzai/filter/include_filter.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'glfm_markdown' +require 'gitlab-glfm-markdown' module Banzai module Filter diff --git a/lib/banzai/filter/markdown_engines/glfm_markdown.rb b/lib/banzai/filter/markdown_engines/glfm_markdown.rb index 01bdbab94a4697..39383d7b7028da 100644 --- a/lib/banzai/filter/markdown_engines/glfm_markdown.rb +++ b/lib/banzai/filter/markdown_engines/glfm_markdown.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -require 'glfm_markdown' +require 'gitlab-glfm-markdown' -# Use the glfm_markdown gem (https://gitlab.com/gitlab-org/ruby/gems/gitlab-glfm-markdown) -# to interface with the Rust based `comrak` parser +# Use the gitlab-glfm-markdown gem (https://gitlab.com/gitlab-org/ruby/gems/gitlab-glfm-markdown) +# to interface with the Rust based Comrak parser # https://github.com/kivikakk/comrak module Banzai module Filter @@ -23,6 +23,7 @@ class GlfmMarkdown < Base full_info_string: true, github_pre_lang: true, hardbreaks: false, + header_accessibility: true, header_ids: Banzai::Renderer::USER_CONTENT_ID_PREFIX, math_code: true, math_dollars: true, diff --git a/lib/banzai/filter/markdown_filter.rb b/lib/banzai/filter/markdown_filter.rb index e6827d5d1c2343..78bef87b46bceb 100644 --- a/lib/banzai/filter/markdown_filter.rb +++ b/lib/banzai/filter/markdown_filter.rb @@ -3,7 +3,7 @@ module Banzai module Filter class MarkdownFilter < HTML::Pipeline::TextFilter - GLFM_ENGINE = :glfm_markdown # glfm_markdown/comrak + GLFM_ENGINE = :glfm_markdown # gitlab-glfm-markdown/Comrak DEFAULT_ENGINE = GLFM_ENGINE def initialize(text, context = nil, result = nil) diff --git a/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb b/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb index 0b658c6484ec3b..57694055e135b2 100644 --- a/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb +++ b/spec/lib/banzai/filter/markdown_engines/glfm_markdown_spec.rb @@ -5,54 +5,52 @@ RSpec.describe Banzai::Filter::MarkdownEngines::GlfmMarkdown, feature_category: :markdown do it 'defaults to generating sourcepos' do engine = described_class.new({}) - expected = <<~TEXT -

hi

- TEXT + html = engine.render('# hi') - expect(engine.render('# hi')).to eq expected + fragment = Nokogiri::HTML.fragment(html) + expect(fragment.css('h1').first['data-sourcepos']).to eq('1:1-1:4') end it 'turns off sourcepos' do engine = described_class.new({ no_sourcepos: true }) - expected = <<~TEXT -

hi

- TEXT + html = engine.render('# hi') - expect(engine.render('# hi')).to eq expected + fragment = Nokogiri::HTML.fragment(html) + expect(fragment.css('h1').first['data-sourcepos']).to be_nil end it 'turns off header anchors' do engine = described_class.new({ no_header_anchors: true, no_sourcepos: true }) - expected = <<~TEXT + expected = <<~HTML

hi

- TEXT + HTML expect(engine.render('# hi')).to eq expected end it 'turns off autolinking' do engine = described_class.new({ autolink: false, no_sourcepos: true }) - expected = <<~TEXT + expected = <<~HTML

http://example.com

- TEXT + HTML expect(engine.render('http://example.com')).to eq expected end it 'returns proper inline sourcepos' do engine = described_class.new({}) - expected = <<~TEXT + expected = <<~HTML

code

- TEXT + HTML expect(engine.render('`code`')).to eq expected end it 'turns on minimal markdown options' do engine = described_class.new({ minimum_markdown: true }) - expected = <<~TEXT + expected = <<~HTML

http://example.com emphasis $x + y$

- TEXT + HTML expect(engine.render('http://example.com _emphasis_ $x + y$')).to eq expected end @@ -66,9 +64,9 @@ shared_examples 'enables placeholder rendering by default' do it 'processes %{} syntax as placeholders' do engine = described_class.new({ project: project_reference, no_sourcepos: true }) - expected = <<~TEXT + expected = <<~HTML

%{test}

- TEXT + HTML expect(engine.render('%{test}')).to eq expected end @@ -84,18 +82,18 @@ it 'turns off placeholder detection when :disable_placeholders' do engine = described_class.new({ disable_placeholders: true, project: project, no_sourcepos: true }) - expected = <<~TEXT + expected = <<~HTML

%{test}

- TEXT + HTML expect(engine.render('%{test}')).to eq expected end it 'turns off placeholder detection when :broadcast_message_placeholders' do engine = described_class.new({ broadcast_message_placeholders: true, project: project, no_sourcepos: true }) - expected = <<~TEXT + expected = <<~HTML

%{test}

- TEXT + HTML expect(engine.render('%{test}')).to eq expected end @@ -104,9 +102,9 @@ stub_feature_flags(markdown_placeholders: false) engine = described_class.new({ project: project, no_sourcepos: true }) - expected = <<~TEXT + expected = <<~HTML

%{test}

- TEXT + HTML expect(engine.render('%{test}')).to eq expected end -- GitLab From abc0f1be86a1d457c9546cde04babd58f7d60830 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Thu, 13 Nov 2025 14:43:34 +1100 Subject: [PATCH 2/9] Other updates for gitlab-glfm-markdown 0.0.39 --- .../filter/markdown_engines/glfm_markdown.rb | 4 +-- lib/gitlab/string_placeholder_replacer.rb | 2 +- .../user_views_open_merge_request_spec.rb | 2 +- .../filter/placeholders_post_filter_spec.rb | 35 +++++++++---------- .../string_placeholder_replacer_spec.rb | 4 +-- 5 files changed, 22 insertions(+), 25 deletions(-) diff --git a/lib/banzai/filter/markdown_engines/glfm_markdown.rb b/lib/banzai/filter/markdown_engines/glfm_markdown.rb index 39383d7b7028da..fc891d9919bfa2 100644 --- a/lib/banzai/filter/markdown_engines/glfm_markdown.rb +++ b/lib/banzai/filter/markdown_engines/glfm_markdown.rb @@ -3,8 +3,7 @@ require 'gitlab-glfm-markdown' # Use the gitlab-glfm-markdown gem (https://gitlab.com/gitlab-org/ruby/gems/gitlab-glfm-markdown) -# to interface with the Rust based Comrak parser -# https://github.com/kivikakk/comrak +# to interface with the Rust-based Comrak parser (https://github.com/kivikakk/comrak). module Banzai module Filter module MarkdownEngines @@ -37,6 +36,7 @@ class GlfmMarkdown < Base table: true, tagfilter: false, tasklist: false, # still handled by a banzai filter/gem, + taskfilter_in_table: false, wikilinks_title_before_pipe: true, unsafe: true }.freeze diff --git a/lib/gitlab/string_placeholder_replacer.rb b/lib/gitlab/string_placeholder_replacer.rb index 3f2b66e96d6832..c5181acef0b8d1 100644 --- a/lib/gitlab/string_placeholder_replacer.rb +++ b/lib/gitlab/string_placeholder_replacer.rb @@ -17,7 +17,7 @@ def self.replace_string_placeholders(string, placeholder_regex = nil, limit: 0, end def self.placeholder_full_regex(placeholder_regex) - /%(\{|%7B)(#{placeholder_regex})(\}|%7D)/ + /%(\{|25%7B)(#{placeholder_regex})(\}|%7D)/ end class << self diff --git a/spec/features/merge_request/user_views_open_merge_request_spec.rb b/spec/features/merge_request/user_views_open_merge_request_spec.rb index 3c2bc7a2be3a66..c3d2dcd39df9b0 100644 --- a/spec/features/merge_request/user_views_open_merge_request_spec.rb +++ b/spec/features/merge_request/user_views_open_merge_request_spec.rb @@ -15,7 +15,7 @@ end it 'renders both the title and the description' do - node = find('.md h1 a#user-content-description-header') + node = find('.md h1#user-content-description-header a') expect(node[:href]).to end_with('#description-header') # Work around a weird Capybara behavior where calling `parent` on a node diff --git a/spec/lib/banzai/filter/placeholders_post_filter_spec.rb b/spec/lib/banzai/filter/placeholders_post_filter_spec.rb index cbde1df2cdb91e..47bb1899eae992 100644 --- a/spec/lib/banzai/filter/placeholders_post_filter_spec.rb +++ b/spec/lib/banzai/filter/placeholders_post_filter_spec.rb @@ -284,28 +284,25 @@ def expect_replaces_placeholder(markdown, expected) it 'does not allow project_title in a link href' do markdown = '[test](%{gitlab_server}/%{project_title}/foo.png)' expected = '

test

' + 'data-placeholder="%25%7Bgitlab_server%7D/%25%7Bproject_title%7D/foo.png" ' \ + 'data-canonical-src="%25%7Bgitlab_server%7D/%25%7Bproject_title%7D/foo.png">test

' expect(run_pipeline(markdown)).to eq expected end it 'does not recognize unknown placeholder in a link href' do markdown = '[test](%{gitlab_server}/%{foo}/foo.png)' - expected = '

test

' + expected = '

test

' expect(run_pipeline(markdown)).to eq expected end - it 'does not allow placeholders in link text (parser limitation)' do + it 'allows placeholders in link text' do markdown = '[%{gitlab_server}](%{gitlab_server})' - expected = '

%{gitlab_server}

' + expected = '

localhost

' expect(run_pipeline(markdown)).to eq expected end @@ -336,9 +333,9 @@ def expect_replaces_placeholder(markdown, expected) it 'generates the correct attributes' do markdown = '![](https://%{gitlab_server}/%{project_name}/foo.png)' expected = "

' + 'data-canonical-src="https://%25%7Bgitlab_server%7D/%25%7Bproject_name%7D/foo.png">

' # can't use the pipeline because the image_link_filter modifies `src` expect(run_filter(markdown)).to eq expected @@ -347,9 +344,9 @@ def expect_replaces_placeholder(markdown, expected) it 'does not allow project_title in a src' do markdown = '![](https://%{gitlab_server}/%{project_title}/foo.png)' expected = "

' + 'data-canonical-src="https://%25%7Bgitlab_server%7D/%25%7Bproject_title%7D/foo.png">

' expect(run_filter(markdown)).to eq expected end @@ -359,9 +356,9 @@ def expect_replaces_placeholder(markdown, expected) markdown = '![](https://%{gitlab_server}/%{project_name}/foo.png)' expected = "

' + 'data-canonical-src="https://%25%7Bgitlab_server%7D/%25%7Bproject_name%7D/foo.png">

' expect(run_filter(markdown)).to eq expected end @@ -379,7 +376,7 @@ def expect_replaces_placeholder(markdown, expected) expect(result).to include "href=\"https://#{Gitlab.config.gitlab.host}/#{project.path}/foo.png\"" expect(result).to include "data-src=\"https://#{Gitlab.config.gitlab.host}/#{project.path}/foo.png\"" - expect(result).to include 'data-canonical-src="https://%%7Bgitlab_server%7D/%%7Bproject_name%7D/foo.png"' + expect(result).to include 'data-canonical-src="https://%25%7Bgitlab_server%7D/%25%7Bproject_name%7D/foo.png"' expect(result) .to include ' Date: Tue, 23 Sep 2025 12:50:34 +1000 Subject: [PATCH 3/9] Show old-style anchors on cached renders The old anchors can be differentiated by their use of `aria-hidden="true"`, so we use the old rules when such an anchor is found. --- .../stylesheets/framework/typography.scss | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index b212416cf04790..b6cc04642af1c3 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -679,20 +679,47 @@ h5, h6 { a.anchor { - @apply gl-text-default gl-no-underline; + &:not([aria-hidden=true]) { + @apply gl-text-default gl-no-underline; + + &:hover, &:focus-visible { + &::after { + @apply gl-inline-block gl-bg-cover; + @include gl-dark-invert-keep-hue; + content: ""; + background-image: url('icon_anchor.svg'); + height: 0.9em; + width: 0.9em; + margin-inline-start: 0.2em; + } + } + } + + &:is([aria-hidden=true]) { + $anchor-offset: 20px; + + margin-left: calc(-1 * $anchor-offset); + text-decoration: none; + outline: none; + position: absolute; + width: $anchor-offset; - &:hover, &:focus-visible { &::after { - @apply gl-inline-block gl-bg-cover; @include gl-dark-invert-keep-hue; - content: ""; - background-image: url('icon_anchor.svg'); - height: 0.9em; - width: 0.9em; - margin-inline-start: 0.2em; + content: url('icon_anchor.svg'); + visibility: hidden; } } } + + &:hover > a.anchor[aria-hidden=true]::after { + visibility: visible; + } + + > a.anchor[aria-hidden=true]:focus::after { + visibility: visible; + outline: auto; + } } .big { -- GitLab From 7a3c451bed6c1a6f86cbf454aef87c5f9e8a155d Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Thu, 25 Sep 2025 15:50:35 +1000 Subject: [PATCH 4/9] CI fixes. --- .../helpers/vulnerabilities_helper_spec.rb | 4 +- .../graphql/vulnerabilities/fields_spec.rb | 14 ++--- lib/banzai/filter/sanitization_filter.rb | 54 ++++++++++--------- .../incidents/user_views_incident_spec.rb | 2 +- spec/features/issues/user_views_issue_spec.rb | 2 +- spec/helpers/markup_helper_spec.rb | 3 +- .../banzai/filter/sanitization_filter_spec.rb | 17 ++---- .../table_of_contents_tag_filter_spec.rb | 2 +- spec/support/matchers/issuable_matchers.rb | 2 +- 9 files changed, 49 insertions(+), 51 deletions(-) diff --git a/ee/spec/helpers/vulnerabilities_helper_spec.rb b/ee/spec/helpers/vulnerabilities_helper_spec.rb index c7e351500cd74e..2b64d08c688ee8 100644 --- a/ee/spec/helpers/vulnerabilities_helper_spec.rb +++ b/ee/spec/helpers/vulnerabilities_helper_spec.rb @@ -603,7 +603,7 @@ end it 'returns finding information' do - rendered_markdown = '

Finding

' + rendered_markdown = '

Finding

' expect(subject[:description_html]).to eq(rendered_markdown) end @@ -616,7 +616,7 @@ end it 'returns finding information' do - rendered_markdown = '

Vulnerability

' + rendered_markdown = '

Vulnerability

' expect(subject[:description_html]).to eq(rendered_markdown) end diff --git a/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb index d4be9ecbe7827a..356051f288f535 100644 --- a/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb +++ b/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb @@ -70,9 +70,10 @@ let(:finding_description) { '# Finding' } it 'returns finding information' do - html = '

' \ - '' \ - 'Finding

' + html = '

' \ + 'Finding' \ + '' \ + '

' expect(subject.first['description']).to eq('# Finding') expect(subject.first['descriptionHtml']).to eq(html) @@ -84,9 +85,10 @@ let(:finding_description) { '# Finding' } it 'returns vulnerability information' do - html = '

' \ - '' \ - 'Vulnerability

' + html = '

' \ + 'Vulnerability' \ + '' \ + '

' expect(subject.first['description']).to eq('# Vulnerability') expect(subject.first['descriptionHtml']).to eq(html) diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index b64ce6520d09fb..480b61c8efb5e6 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -9,6 +9,7 @@ class SanitizationFilter < Banzai::Filter::BaseSanitizationFilter # Styles used by Markdown for table alignment TABLE_ALIGNMENT_PATTERN = /text-align: (?center|left|right)/ ALLOWED_IDIFF_CLASSES = %w[idiff left right deletion addition].freeze + HEADER_NODE_NAMES = %w[h1 h2 h3 h4 h5 h6].freeze def customize_allowlist(allowlist) allowlist[:allow_comments] = context[:allow_comments] @@ -32,7 +33,7 @@ def allow_table_alignment(allowlist) allowlist[:css] = { properties: ['text-align'] } # Remove any `style` properties not required for table alignment - allowlist[:transformers].push(self.class.remove_unsafe_table_style) + allowlist[:transformers].push(self.class.method(:remove_unsafe_table_style)) end def allow_json_table_attributes(allowlist) @@ -47,11 +48,13 @@ def allow_sourcepos_and_escaped_char(allowlist) end def allow_id_attributes(allowlist) - # Allow `id` in `a` and `li` elements for footnotes and `a` elements for header anchors. + # Allow `id` in `a` and `li` elements for footnotes and `h1`~`h6` elements for header anchors. # Remove any `id` properties not matching these patterns via transformer allowlist[:attributes]['a'].push('id') - allowlist[:attributes]['li'] = %w[id] - allowlist[:transformers].push(self.class.remove_id_attributes) + (["li"] + HEADER_NODE_NAMES).each do |tag| + allowlist[:attributes][tag] = %w[id] + end + allowlist[:transformers].push(self.class.method(:remove_id_attributes)) end def allow_class_attributes(allowlist) @@ -61,7 +64,7 @@ def allow_class_attributes(allowlist) allowlist[:attributes]['p'] = %w[class] allowlist[:attributes]['span'].push('class') allowlist[:attributes]['code'].push('class') - allowlist[:transformers].push(self.class.remove_unsafe_classes) + allowlist[:transformers].push(self.class.method(:remove_unsafe_classes)) end def allow_section_footnotes(allowlist) @@ -72,8 +75,7 @@ def allow_section_footnotes(allowlist) end class << self - def remove_unsafe_table_style - ->(env) do + def remove_unsafe_table_style(env) node = env[:node] return unless node.name == 'th' || node.name == 'td' @@ -84,11 +86,9 @@ def remove_unsafe_table_style else node.remove_attribute('style') end - end end - def remove_unsafe_classes - ->(env) do + def remove_unsafe_classes(env) node = env[:node] return unless node.has_attribute?('class') @@ -105,7 +105,6 @@ def remove_unsafe_classes when 'code' node.remove_attribute('class') if remove_code_class?(node) end - end end def remove_link_class?(node) @@ -134,24 +133,27 @@ def remove_code_class?(node) node['class'] != 'idiff' end - def remove_id_attributes - ->(env) do - node = env[:node] - - return unless node.name == 'a' || node.name == 'li' - return unless node.has_attribute?('id') + def remove_id_attributes(env) + node = env[:node] + return unless node.has_attribute?('id') - # footnote ids should not be removed - return if node.name == 'li' && node['id'].start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_ID_PREFIX) - return if node.name == 'a' && - node['id'].start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_ID_PREFIX) + id = node['id'] - # links with generated header anchors should not be removed - return if node.name == 'a' && node['class'] == 'anchor' && - node['id'].start_with?(Banzai::Renderer::USER_CONTENT_ID_PREFIX) - - node.remove_attribute('id') + case node.name + when 'a' + # footnote ids should not be removed + return if id.start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_ID_PREFIX) + when 'li' + # footnote ids should not be removed + return if id.start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_ID_PREFIX) + when *HEADER_NODE_NAMES + # headers with generated header anchors should not be removed + return if id.start_with?(Banzai::Renderer::USER_CONTENT_ID_PREFIX) + else + return end + + node.remove_attribute('id') end end end diff --git a/spec/features/incidents/user_views_incident_spec.rb b/spec/features/incidents/user_views_incident_spec.rb index 0626178cd11d14..720cc6309c8fd9 100644 --- a/spec/features/incidents/user_views_incident_spec.rb +++ b/spec/features/incidents/user_views_incident_spec.rb @@ -27,7 +27,7 @@ expect(page).to have_header_with_correct_id_and_link(1, 'Description header', 'description-header') end - it_behaves_like 'page meta description', ' Description header Lorem ipsum dolor sit amet' + it_behaves_like 'page meta description', 'Description header Lorem ipsum dolor sit amet' describe 'user actions' do it 'shows the merge request and incident actions', :js, :aggregate_failures do diff --git a/spec/features/issues/user_views_issue_spec.rb b/spec/features/issues/user_views_issue_spec.rb index ee3f6836b083ad..e088f6b8fb64e1 100644 --- a/spec/features/issues/user_views_issue_spec.rb +++ b/spec/features/issues/user_views_issue_spec.rb @@ -16,7 +16,7 @@ it { expect(page).to have_header_with_correct_id_and_link(1, "Description header", "description-header") } - it_behaves_like 'page meta description', ' Description header Lorem ipsum dolor sit amet' + it_behaves_like 'page meta description', 'Description header Lorem ipsum dolor sit amet' it 'shows the merge request and issue actions', :aggregate_failures do click_button 'More actions', match: :first diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index d196172a3b88d9..7f4bb3be9c107a 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -397,7 +397,8 @@ result = helper.render_wiki_content(wiki_page) - expect(result).to include('Header') + expect(result).to include('>Header ') end end diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb index aaa804079b2bde..ad1c084bca8678 100644 --- a/spec/lib/banzai/filter/sanitization_filter_spec.rb +++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb @@ -222,24 +222,17 @@ end end - describe 'link anchors' do - it 'allows id property on anchor links' do - exp = %q() + describe 'heading anchors' do + it 'allows id property on headings' do + exp = %q(

) act = filter(exp) expect(act.to_html).to eq exp end - it 'removes id property for non-anchor links' do - exp = %q() - act = filter(%q()) - - expect(act.to_html).to eq exp - end - it 'removes id property for non-user-content links' do - exp = %q() - act = filter(%q()) + act = filter(%q(

)) + exp = %q(

) expect(act.to_html).to eq exp end diff --git a/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb index 6eee6950f1faf7..b996d2112e4059 100644 --- a/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_tag_filter_spec.rb @@ -69,7 +69,7 @@ doc = pipeline_filter("this [[_toc_]]\n\n# Foo") expect(doc.to_html).to include('this _toc_') - expect(doc.to_html).to include('Foo') + expect(doc.to_html).to include('>Foo Date: Mon, 13 Oct 2025 17:57:25 +1100 Subject: [PATCH 5/9] Rework page meta matchers to show their working on failure Just having "element not found" is not so helpful if you're trying to work out why. --- .../features/page_description_shared_examples.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/support/shared_examples/features/page_description_shared_examples.rb b/spec/support/shared_examples/features/page_description_shared_examples.rb index 163c65915ba82b..7441630dbf7eb4 100644 --- a/spec/support/shared_examples/features/page_description_shared_examples.rb +++ b/spec/support/shared_examples/features/page_description_shared_examples.rb @@ -3,7 +3,8 @@ RSpec.shared_examples 'page meta description' do |expected_description| it 'renders the page with description, og:description, and twitter:description meta tags that contains a plain-text version of the markdown', :aggregate_failures do %w[name='description' property='og:description' property='twitter:description'].each do |selector| - expect(page).to have_selector("meta[#{selector}][content='#{expected_description}']", visible: false) + node = find("meta[#{selector}]", visible: false) + expect(node['content']).to eq(expected_description) end end end @@ -13,7 +14,8 @@ it 'renders the page with description, og:description, and twitter:description meta tags with the default brand title', :aggregate_failures do %w[name='description' property='og:description' property='twitter:description'].each do |selector| - expect(page).to have_selector("meta[#{selector}][content='#{default_brand_title}']", visible: false) + node = find("meta[#{selector}]", visible: false) + expect(node['content']).to eq(default_brand_title) end end end -- GitLab From b1a923d8beaa10ad2507922458b8c599a8a97efa Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Tue, 18 Nov 2025 16:26:13 +1100 Subject: [PATCH 6/9] Show anchors on hover --- .../stylesheets/framework/typography.scss | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index b6cc04642af1c3..38f722becd9428 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -680,18 +680,15 @@ h6 { a.anchor { &:not([aria-hidden=true]) { - @apply gl-text-default gl-no-underline; - - &:hover, &:focus-visible { - &::after { - @apply gl-inline-block gl-bg-cover; - @include gl-dark-invert-keep-hue; - content: ""; - background-image: url('icon_anchor.svg'); - height: 0.9em; - width: 0.9em; - margin-inline-start: 0.2em; - } + &::after { + @apply gl-inline-block gl-bg-cover; + @include gl-dark-invert-keep-hue; + content: ""; + background-image: url('icon_anchor.svg'); + height: 0.9em; + width: 0.9em; + margin-inline-start: 0.2em; + visibility: hidden; } } @@ -712,11 +709,11 @@ } } - &:hover > a.anchor[aria-hidden=true]::after { + &:hover > a.anchor::after { visibility: visible; } - > a.anchor[aria-hidden=true]:focus::after { + > a.anchor:focus::after { visibility: visible; outline: auto; } -- GitLab From 5af00b6e52e160a597f2f0c89c239e74f58568d0 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Tue, 18 Nov 2025 16:29:46 +1100 Subject: [PATCH 7/9] Localise the heading anchor aria-label --- .../helpers/vulnerabilities_helper_spec.rb | 4 +- .../graphql/vulnerabilities/fields_spec.rb | 4 +- .../filter/heading_accessibility_filter.rb | 41 +++++++++++++++++++ lib/banzai/filter/sanitization_filter.rb | 5 +++ lib/banzai/pipeline/gfm_pipeline.rb | 1 + locale/gitlab.pot | 3 ++ .../heading_accessibility_filter_spec.rb | 39 ++++++++++++++++++ 7 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 lib/banzai/filter/heading_accessibility_filter.rb create mode 100644 spec/lib/banzai/filter/heading_accessibility_filter_spec.rb diff --git a/ee/spec/helpers/vulnerabilities_helper_spec.rb b/ee/spec/helpers/vulnerabilities_helper_spec.rb index 2b64d08c688ee8..7a3b914142235f 100644 --- a/ee/spec/helpers/vulnerabilities_helper_spec.rb +++ b/ee/spec/helpers/vulnerabilities_helper_spec.rb @@ -603,7 +603,7 @@ end it 'returns finding information' do - rendered_markdown = '

Finding

' + rendered_markdown = '

Finding

' expect(subject[:description_html]).to eq(rendered_markdown) end @@ -616,7 +616,7 @@ end it 'returns finding information' do - rendered_markdown = '

Vulnerability

' + rendered_markdown = '

Vulnerability

' expect(subject[:description_html]).to eq(rendered_markdown) end diff --git a/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb index 356051f288f535..017d0f5dd31b10 100644 --- a/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb +++ b/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb @@ -72,7 +72,7 @@ it 'returns finding information' do html = '

' \ 'Finding' \ - '' \ + '' \ '

' expect(subject.first['description']).to eq('# Finding') @@ -87,7 +87,7 @@ it 'returns vulnerability information' do html = '

' \ 'Vulnerability' \ - '' \ + '' \ '

' expect(subject.first['description']).to eq('# Vulnerability') diff --git a/lib/banzai/filter/heading_accessibility_filter.rb b/lib/banzai/filter/heading_accessibility_filter.rb new file mode 100644 index 00000000000000..fdf90b825d006a --- /dev/null +++ b/lib/banzai/filter/heading_accessibility_filter.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Banzai + module Filter + # Finds heading anchors and localises the accessible labels on them. + # + # Headings with anchors as generated by our Markdown parser look as follows. + # Given the input: + # + # ```markdown + # ## so _cool_ h2 + # ``` + # + # The HTML generated is (newlines and spaces added for readability): + # + # ```html + #

+ # so cool h2 + # + #

+ # ``` + # + # We use [data-heading-content] to rewrite aria-label in a localisable way. + class HeadingAccessibilityFilter < HTML::Pipeline::Filter + prepend Concerns::PipelineTimingCheck + + SUBANCHOR_CSS = '> a:last-child.anchor[aria-label][data-heading-content]' + ANCHOR_CSS = %w[h1 h2 h3 h4 h5 h6].map { |h| "#{h} #{SUBANCHOR_CSS}" }.join(', ') + ANCHOR_XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(ANCHOR_CSS).freeze + + def call + doc.xpath(ANCHOR_XPATH).each do |node| + node['aria-label'] = format(_("Link to heading '%{heading}'"), heading: node['data-heading-content']) + end + + doc + end + end + end +end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index 480b61c8efb5e6..532090063b2825 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -20,6 +20,7 @@ def customize_allowlist(allowlist) allow_id_attributes(allowlist) allow_class_attributes(allowlist) allow_section_footnotes(allowlist) + allow_anchor_data_heading_content(allowlist) allowlist end @@ -74,6 +75,10 @@ def allow_section_footnotes(allowlist) allowlist[:attributes]['a'].push('data-footnote-ref', 'data-footnote-backref', 'data-footnote-backref-idx') end + def allow_anchor_data_heading_content(allowlist) + allowlist[:attributes]['a'].push('data-heading-content') + end + class << self def remove_unsafe_table_style(env) node = env[:node] diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index b9ef83eb498fb4..60b87273231e5a 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -32,6 +32,7 @@ def self.filters Filter::VideoLinkFilter, Filter::AudioLinkFilter, Filter::IframeLinkFilter, + Filter::HeadingAccessibilityFilter, Filter::TableOfContentsTagFilter, Filter::AutolinkFilter, Filter::SuggestionFilter, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 902262e2c95822..9200310730a68e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -39373,6 +39373,9 @@ msgstr "" msgid "Link to go to GitLab pipeline documentation" msgstr "" +msgid "Link to heading '%{heading}'" +msgstr "" + msgid "Link to your Grafana instance." msgstr "" diff --git a/spec/lib/banzai/filter/heading_accessibility_filter_spec.rb b/spec/lib/banzai/filter/heading_accessibility_filter_spec.rb new file mode 100644 index 00000000000000..a9ad714e2a9fd4 --- /dev/null +++ b/spec/lib/banzai/filter/heading_accessibility_filter_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::Filter::HeadingAccessibilityFilter, feature_category: :markdown do + include FilterSpecHelper + + def filter(markdown) + Banzai::PipelineBase.new([Banzai::Filter::MarkdownFilter, described_class]).call(markdown)[:output] + end + + def pipeline_filter(text, context = {}) + context = { project: nil, no_sourcepos: true }.merge(context) + + doc = Banzai::Pipeline::PreProcessPipeline.call(text, {}) + doc = Banzai::Pipeline::FullPipeline.call(doc[:output], context) + + doc[:output] + end + + it 'rewrites heading aria-labels' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:_).and_return("Link pealkirjale '%{heading}'") + end + doc = filter("# Tere, maailm!") + a = doc.css('h1 > a.anchor').first + expect(a['aria-label']).to eq("Link pealkirjale 'Tere, maailm!'") + end + + # Check that (a) it actually runs, and (b) the needed attributes aren't sanitised out. + it 'rewrites heading aria-labels in the full pipeline' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:_).and_return("Link pealkirjale '%{heading}'") + end + doc = pipeline_filter("# Tere, maailm!") + a = doc.css('h1 > a.anchor').first + expect(a['aria-label']).to eq("Link pealkirjale 'Tere, maailm!'") + end +end -- GitLab From 7fc3b22679ac42523c908953caa2a843797d8bbc Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Tue, 18 Nov 2025 17:53:22 +1100 Subject: [PATCH 8/9] Static analysis fixes --- .../stylesheets/framework/typography.scss | 3 +- .../graphql/vulnerabilities/fields_spec.rb | 24 ++++--- lib/banzai/filter/sanitization_filter.rb | 70 +++++++++---------- 3 files changed, 51 insertions(+), 46 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 38f722becd9428..0f2a2f752ad91c 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -681,7 +681,8 @@ a.anchor { &:not([aria-hidden=true]) { &::after { - @apply gl-inline-block gl-bg-cover; + @apply gl-inline-block; + @apply gl-bg-cover; @include gl-dark-invert-keep-hue; content: ""; background-image: url('icon_anchor.svg'); diff --git a/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb index 017d0f5dd31b10..c4e50e90041c0b 100644 --- a/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb +++ b/ee/spec/requests/api/graphql/vulnerabilities/fields_spec.rb @@ -70,13 +70,15 @@ let(:finding_description) { '# Finding' } it 'returns finding information' do - html = '

' \ - 'Finding' \ - '' \ - '

' + html = %(

+ Finding + + +

) expect(subject.first['description']).to eq('# Finding') - expect(subject.first['descriptionHtml']).to eq(html) + expect(subject.first['descriptionHtml']).to eq_html(html, trim_text_nodes: true) end end @@ -85,13 +87,15 @@ let(:finding_description) { '# Finding' } it 'returns vulnerability information' do - html = '

' \ - 'Vulnerability' \ - '' \ - '

' + html = %(

+ Vulnerability + + +

) expect(subject.first['description']).to eq('# Vulnerability') - expect(subject.first['descriptionHtml']).to eq(html) + expect(subject.first['descriptionHtml']).to eq_html(html, trim_text_nodes: true) end end end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index 532090063b2825..5e768163716a55 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -81,35 +81,35 @@ def allow_anchor_data_heading_content(allowlist) class << self def remove_unsafe_table_style(env) - node = env[:node] + node = env[:node] - return unless node.name == 'th' || node.name == 'td' - return unless node.has_attribute?('style') + return unless node.name == 'th' || node.name == 'td' + return unless node.has_attribute?('style') - if node['style'] =~ TABLE_ALIGNMENT_PATTERN - node['style'] = "text-align: #{$~[:alignment]}" - else - node.remove_attribute('style') - end + if node['style'] =~ TABLE_ALIGNMENT_PATTERN + node['style'] = "text-align: #{$~[:alignment]}" + else + node.remove_attribute('style') + end end def remove_unsafe_classes(env) - node = env[:node] - - return unless node.has_attribute?('class') - - case node.name - when 'a' - node.remove_attribute('class') if remove_link_class?(node) - when 'div' - node.remove_attribute('class') if remove_div_class?(node) - when 'p' - node.remove_attribute('class') if remove_p_class?(node) - when 'span' - node.remove_attribute('class') if remove_span_class?(node) - when 'code' - node.remove_attribute('class') if remove_code_class?(node) - end + node = env[:node] + + return unless node.has_attribute?('class') + + case node.name + when 'a' + node.remove_attribute('class') if remove_link_class?(node) + when 'div' + node.remove_attribute('class') if remove_div_class?(node) + when 'p' + node.remove_attribute('class') if remove_p_class?(node) + when 'span' + node.remove_attribute('class') if remove_span_class?(node) + when 'code' + node.remove_attribute('class') if remove_code_class?(node) + end end def remove_link_class?(node) @@ -145,17 +145,17 @@ def remove_id_attributes(env) id = node['id'] case node.name - when 'a' - # footnote ids should not be removed - return if id.start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_ID_PREFIX) - when 'li' - # footnote ids should not be removed - return if id.start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_ID_PREFIX) - when *HEADER_NODE_NAMES - # headers with generated header anchors should not be removed - return if id.start_with?(Banzai::Renderer::USER_CONTENT_ID_PREFIX) - else - return + when 'a' + # footnote ids should not be removed + return if id.start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_LINK_ID_PREFIX) + when 'li' + # footnote ids should not be removed + return if id.start_with?(Banzai::Filter::FootnoteFilter::FOOTNOTE_ID_PREFIX) + when *HEADER_NODE_NAMES + # headers with generated header anchors should not be removed + return if id.start_with?(Banzai::Renderer::USER_CONTENT_ID_PREFIX) + else + return end node.remove_attribute('id') -- GitLab From 0c3a32f00938eb773f1c517a302f20428cb49f41 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Thu, 20 Nov 2025 14:51:57 +1100 Subject: [PATCH 9/9] Use utility classes in CSS --- app/assets/stylesheets/framework/typography.scss | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 0f2a2f752ad91c..4ea41798e7b4ae 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -689,7 +689,7 @@ height: 0.9em; width: 0.9em; margin-inline-start: 0.2em; - visibility: hidden; + @apply gl-invisible; } } @@ -697,15 +697,15 @@ $anchor-offset: 20px; margin-left: calc(-1 * $anchor-offset); - text-decoration: none; - outline: none; - position: absolute; + @apply gl-no-underline; + @apply gl-outline-none; + @apply gl-absolute; width: $anchor-offset; &::after { @include gl-dark-invert-keep-hue; content: url('icon_anchor.svg'); - visibility: hidden; + @apply gl-invisible; } } } -- GitLab