diff --git a/app/assets/stylesheets/lightbox.css b/app/assets/stylesheets/lightbox.css index 4a9cb3d8b..65144a22b 100644 --- a/app/assets/stylesheets/lightbox.css +++ b/app/assets/stylesheets/lightbox.css @@ -67,7 +67,7 @@ } .lightbox__caption { - color: var(--color-ink-inverted); + color: var(--color-white); &:empty { display: none; diff --git a/app/javascript/controllers/lightbox_controller.js b/app/javascript/controllers/lightbox_controller.js index e8bc48f03..fc86b18f7 100644 --- a/app/javascript/controllers/lightbox_controller.js +++ b/app/javascript/controllers/lightbox_controller.js @@ -3,9 +3,22 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { static targets = [ "caption", "image", "dialog", "zoomedImage" ] - open(event) { + imageTargetConnected(element) { + element.addEventListener("click", this.#handleImageClick) + } + + imageTargetDisconnected(element) { + element.removeEventListener("click", this.#handleImageClick) + } + + #handleImageClick = (event) => { + event.preventDefault() + this.#open(event.currentTarget) + } + + #open(link) { this.dialogTarget.showModal() - this.#set(event.target.closest("a")) + this.#set(link) } // Wait for the transition to finish before resetting the image diff --git a/app/views/active_storage/blobs/web/_representation.html.erb b/app/views/active_storage/blobs/web/_representation.html.erb index 2aa1b51a4..ca934f469 100644 --- a/app/views/active_storage/blobs/web/_representation.html.erb +++ b/app/views/active_storage/blobs/web/_representation.html.erb @@ -15,7 +15,7 @@ <% elsif blob.variable? %> - <%= link_to url_for(blob.variant(variant)), data: { action: "lightbox#open:prevent", lightbox_target: "image" } do %> + <%= link_to url_for(blob.variant(variant)), data: { lightbox_target: "image", lightbox_caption_value: blob.filename.to_s } do %> <%= image_tag url_for(blob.variant(variant)), width: width, height: height %> <% end %> <% elsif blob.previewable? %> diff --git a/app/views/cards/display/perma/_background.html.erb b/app/views/cards/display/perma/_background.html.erb index 6ade6f1b5..2315b0118 100644 --- a/app/views/cards/display/perma/_background.html.erb +++ b/app/views/cards/display/perma/_background.html.erb @@ -1,5 +1,5 @@ <%= render "cards/display/common/background", card: card do %> - <%= link_to card.image.presence, class: "btn fill-transparent flex-item-justify-end", data: { controller: "tooltip", action: "lightbox#open:prevent", lightbox_target: "image" } do %> + <%= link_to card.image.presence, class: "btn fill-transparent flex-item-justify-end", data: { controller: "tooltip", lightbox_target: "image" } do %> <%= icon_tag "picture-zoom" %> Zoom background image <% end %> diff --git a/config/initializers/sanitization.rb b/config/initializers/sanitization.rb index 6fae14b25..0f2325eff 100644 --- a/config/initializers/sanitization.rb +++ b/config/initializers/sanitization.rb @@ -1,6 +1,6 @@ Rails.application.config.after_initialize do Rails::HTML5::SafeListSanitizer.allowed_tags.merge(%w[ s table tr td th thead tbody details summary video source ]) - Rails::HTML5::SafeListSanitizer.allowed_attributes.merge(%w[ data-turbo-frame controls type width data-action data-lightbox-target data-lightbox-url-value ]) + Rails::HTML5::SafeListSanitizer.allowed_attributes.merge(%w[ data-turbo-frame data-lightbox-target data-lightbox-caption-value controls type width ]) # ugh, see https://github.com/rails/rails/issues/54478 which I need to fix upstream --mike ActionText::ContentHelper.allowed_tags = Rails::HTML5::SafeListSanitizer.allowed_tags.to_a + [ ActionText::Attachment.tag_name, "figure", "figcaption" ] + ActionText::ContentHelper.allowed_tags.to_a diff --git a/test/helpers/action_text_rendering_test.rb b/test/helpers/action_text_rendering_test.rb new file mode 100644 index 000000000..6cb8fcc94 --- /dev/null +++ b/test/helpers/action_text_rendering_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class ActionTextRenderingTest < ActionView::TestCase + test "data-action attributes in user content are stripped" do + malicious_html = <<~HTML +

Click here: malicious link

+ HTML + + content = ActionText::Content.new(malicious_html) + rendered = content.to_s + + assert_no_match(/data-action/, rendered) + assert_match(/malicious link<\/a>/, rendered) + end +end diff --git a/test/system/smoke_test.rb b/test/system/smoke_test.rb index 14c7a7116..c307c50d2 100644 --- a/test/system/smoke_test.rb +++ b/test/system/smoke_test.rb @@ -54,6 +54,14 @@ class SmokeTest < ApplicationSystemTestCase assert_selector "a img[src*='/rails/active_storage']" assert_selector "figcaption span.attachment__name", text: "moon.jpg" end + + # Click the image to open the lightbox + find("action-text-attachment figure.attachment a:has(img)").click + + assert_selector "dialog.lightbox[open]" + within("dialog.lightbox") do + assert_selector "img.lightbox__image[src*='/rails/active_storage']" + end end test "dismissing notifications" do