Add animate custom emoji param to embed pages (#8507)

* Add animate custom emoji param to embed pages

* Rename param, use it for avatars and gifs

* Fix issues pointed by codeclimate and breaking test

* Ignore brakeman warning

diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb
index 24824aabb7268e48c8b9eab0e5c9c3398ff92bb4..d4ad3df60194926337fecfb3342a05001371eaa2 100644 (file)
--- a/app/controllers/statuses_controller.rb
+++ b/app/controllers/statuses_controller.rb
@@ -54,6 +54,7 @@ class StatusesController < ApplicationController
     skip_session!
     expires_in 180, public: true
     response.headers['X-Frame-Options'] = 'ALLOWALL'
+    @autoplay = ActiveModel::Type::Boolean.new.cast(params[:autoplay])
 
     render 'stream_entries/embed', layout: 'embedded'
   end

diff --git a/app/lib/formatter.rb b/app/lib/formatter.rb
index e1ab05cc0bf769241b050c54a3ac96a92fcfb93b..0b4690394caf292b7689784eb31229af4d93c00c 100644 (file)
--- a/app/lib/formatter.rb
+++ b/app/lib/formatter.rb
@@ -23,7 +23,7 @@ class Formatter
 
     unless status.local?
       html = reformat(raw_content)
-      html = encode_custom_emojis(html, status.emojis) if options[:custom_emojify]
+      html = encode_custom_emojis(html, status.emojis, options[:autoplay]) if options[:custom_emojify]
       return html.html_safe # rubocop:disable Rails/OutputSafety
     end
 
@@ -33,7 +33,7 @@ class Formatter
     html = raw_content
     html = "RT @#{prepend_reblog} #{html}" if prepend_reblog
     html = encode_and_link_urls(html, linkable_accounts)
-    html = encode_custom_emojis(html, status.emojis) if options[:custom_emojify]
+    html = encode_custom_emojis(html, status.emojis, options[:autoplay]) if options[:custom_emojify]
     html = simple_format(html, {}, sanitize: false)
     html = html.delete("\n")
 
@@ -53,7 +53,7 @@ class Formatter
 
   def simplified_format(account, **options)
     html = account.local? ? linkify(account.note) : reformat(account.note)
-    html = encode_custom_emojis(html, account.emojis) if options[:custom_emojify]
+    html = encode_custom_emojis(html, account.emojis, options[:autoplay]) if options[:custom_emojify]
     html.html_safe # rubocop:disable Rails/OutputSafety
   end
 
@@ -63,20 +63,20 @@ class Formatter
 
   def format_spoiler(status)
     html = encode(status.spoiler_text)
-    html = encode_custom_emojis(html, status.emojis)
+    html = encode_custom_emojis(html, status.emojis, options[:autoplay])
     html.html_safe # rubocop:disable Rails/OutputSafety
   end
 
   def format_display_name(account, **options)
     html = encode(account.display_name.presence || account.username)
-    html = encode_custom_emojis(html, account.emojis) if options[:custom_emojify]
+    html = encode_custom_emojis(html, account.emojis, options[:autoplay]) if options[:custom_emojify]
     html.html_safe # rubocop:disable Rails/OutputSafety
   end
 
   def format_field(account, str, **options)
     return reformat(str).html_safe unless account.local? # rubocop:disable Rails/OutputSafety
     html = encode_and_link_urls(str, me: true)
-    html = encode_custom_emojis(html, account.emojis) if options[:custom_emojify]
+    html = encode_custom_emojis(html, account.emojis, options[:autoplay]) if options[:custom_emojify]
     html.html_safe # rubocop:disable Rails/OutputSafety
   end
 
@@ -120,10 +120,14 @@ class Formatter
     end
   end
 
-  def encode_custom_emojis(html, emojis)
+  def encode_custom_emojis(html, emojis, animate = false)
     return html if emojis.empty?
 
-    emoji_map = emojis.map { |e| [e.shortcode, full_asset_url(e.image.url(:static))] }.to_h
+    emoji_map = if animate
+                  emojis.map { |e| [e.shortcode, full_asset_url(e.image.url)] }.to_h
+                else
+                  emojis.map { |e| [e.shortcode, full_asset_url(e.image.url(:static))] }.to_h
+                end
 
     i                     = -1
     tag_open_index        = nil

diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml
index 7843005c130da64063be7368c33900b107827723..6251cdd1a52a0a0bd826d362520685580ccd56cf 100644 (file)
--- a/app/views/stream_entries/_detailed_status.html.haml
+++ b/app/views/stream_entries/_detailed_status.html.haml
@@ -1,10 +1,13 @@
 .detailed-status.detailed-status--flex
   = link_to TagManager.instance.url_for(status.account), class: 'detailed-status__display-name p-author h-card', target: stream_link_target, rel: 'noopener' do
     .detailed-status__display-avatar
-      = image_tag status.account.avatar.url(:original), width: 48, height: 48, alt: '', class: 'account__avatar u-photo'
+      - if current_account&.user&.setting_auto_play_gif || autoplay
+        = image_tag status.account.avatar.url(:original), width: 48, height: 48, alt: '', class: 'account__avatar u-photo'
+      - else
+        = image_tag status.account.avatar.url(:static), width: 48, height: 48, alt: '', class: 'account__avatar u-photo'
     %span.display-name
       %bdi
-        %strong.display-name__html.p-name.emojify= display_name(status.account, custom_emojify: true)
+        %strong.display-name__html.p-name.emojify= display_name(status.account, custom_emojify: true, autoplay: autoplay)
       %span.display-name__account
         = acct(status.account)
         = fa_icon('lock') if status.account.locked?
@@ -16,14 +19,14 @@
       %p{ style: 'margin-bottom: 0' }<
         %span.p-summary> #{Formatter.instance.format_spoiler(status)}&nbsp;
         %a.status__content__spoiler-link{ href: '#' }= t('statuses.show_more')
-    .e-content{ lang: status.language, style: "display: #{status.spoiler_text? ? 'none' : 'block'}; direction: #{rtl_status?(status) ? 'rtl' : 'ltr'}" }= Formatter.instance.format(status, custom_emojify: true)
+    .e-content{ lang: status.language, style: "display: #{status.spoiler_text? ? 'none' : 'block'}; direction: #{rtl_status?(status) ? 'rtl' : 'ltr'}" }= Formatter.instance.format(status, custom_emojify: true, autoplay: autoplay)
 
   - if !status.media_attachments.empty?
     - if status.media_attachments.first.video?
       - video = status.media_attachments.first
       = react_component :video, src: video.file.url(:original), preview: video.file.url(:small), sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, width: 670, height: 380, detailed: true, inline: true, alt: video.description
     - else
-      = react_component :media_gallery, height: 380, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, standalone: true, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, 'reduceMotion': current_account&.user&.setting_reduce_motion, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }
+      = react_component :media_gallery, height: 380, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, standalone: true, 'autoPlayGif': current_account&.user&.setting_auto_play_gif || autoplay, 'reduceMotion': current_account&.user&.setting_reduce_motion, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }
   - elsif status.preview_cards.first
     = react_component :card, 'maxDescription': 160, card: ActiveModelSerializers::SerializableResource.new(status.preview_cards.first, serializer: REST::PreviewCardSerializer).as_json
 

diff --git a/app/views/stream_entries/_simple_status.html.haml b/app/views/stream_entries/_simple_status.html.haml
index 875795f2a0bc55a3d38a0063eba503f107ad79b6..1d596d4e0a7f243928a3dff920a6bdf2e69fb5a7 100644 (file)
--- a/app/views/stream_entries/_simple_status.html.haml
+++ b/app/views/stream_entries/_simple_status.html.haml
@@ -7,10 +7,13 @@
     = link_to TagManager.instance.url_for(status.account), class: 'status__display-name p-author h-card', target: stream_link_target, rel: 'noopener' do
       .status__avatar
         %div
-          = image_tag status.account.avatar(:original), width: 48, height: 48, alt: '', class: 'u-photo account__avatar'
+          - if current_account&.user&.setting_auto_play_gif || autoplay
+            = image_tag status.account.avatar(:original), width: 48, height: 48, alt: '', class: 'u-photo account__avatar'
+          - else
+            = image_tag status.account.avatar(:static), width: 48, height: 48, alt: '', class: 'u-photo account__avatar'
       %span.display-name
         %bdi
-          %strong.display-name__html.p-name.emojify= display_name(status.account, custom_emojify: true)
+          %strong.display-name__html.p-name.emojify= display_name(status.account, custom_emojify: true, autoplay: autoplay)
         %span.display-name__account
           = acct(status.account)
           = fa_icon('lock') if status.account.locked?
@@ -19,14 +22,14 @@
       %p{ style: 'margin-bottom: 0' }<
         %span.p-summary> #{Formatter.instance.format_spoiler(status)}&nbsp;
         %a.status__content__spoiler-link{ href: '#' }= t('statuses.show_more')
-    .e-content{ lang: status.language, style: "display: #{status.spoiler_text? ? 'none' : 'block'}; direction: #{rtl_status?(status) ? 'rtl' : 'ltr'}" }= Formatter.instance.format(status, custom_emojify: true)
+    .e-content{ lang: status.language, style: "display: #{status.spoiler_text? ? 'none' : 'block'}; direction: #{rtl_status?(status) ? 'rtl' : 'ltr'}" }= Formatter.instance.format(status, custom_emojify: true, autoplay: autoplay)
 
   - unless status.media_attachments.empty?
     - if status.media_attachments.first.video?
       - video = status.media_attachments.first
       = react_component :video, src: video.file.url(:original), preview: video.file.url(:small), sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, width: 610, height: 343, inline: true, alt: video.description
     - else
-      = react_component :media_gallery, height: 343, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }
+      = react_component :media_gallery, height: 343, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, 'autoPlayGif': current_account&.user&.setting_auto_play_gif || autoplay, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }
 
   .status__action-bar
     .status__action-bar__counter

diff --git a/app/views/stream_entries/_status.html.haml b/app/views/stream_entries/_status.html.haml
index 92003a48f370c89969063bee930893bd54de9bf3..83887cd87437f7a73548a5e69062c5cd3038f5da 100644 (file)
--- a/app/views/stream_entries/_status.html.haml
+++ b/app/views/stream_entries/_status.html.haml
@@ -5,6 +5,7 @@
   is_successor    ||= false
   direct_reply_id ||= false
   parent_id       ||= false
+  autoplay        ||= current_account&.user&.setting_auto_play_gif
   is_direct_parent  = direct_reply_id == status.id
   is_direct_child   = parent_id == status.in_reply_to_id
   centered        ||= include_threads && !is_predecessor && !is_successor
@@ -18,7 +19,7 @@
     .entry{ class: entry_classes }
       = link_to_more TagManager.instance.url_for(@next_ancestor)
 
-  = render partial: 'stream_entries/status', collection: @ancestors, as: :status, locals: { is_predecessor: true, direct_reply_id: status.in_reply_to_id }
+  = render partial: 'stream_entries/status', collection: @ancestors, as: :status, locals: { is_predecessor: true, direct_reply_id: status.in_reply_to_id }, autoplay: autoplay
 
 .entry{ class: entry_classes }
 
@@ -38,14 +39,14 @@
       %span
         = t('stream_entries.pinned')
 
-  = render (centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status'), status: status.proper
+  = render (centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status'), status: status.proper, autoplay: autoplay
 
 - if include_threads
   - if @since_descendant_thread_id
     .entry{ class: entry_classes }
       = link_to_more short_account_status_url(status.account.username, status, max_descendant_thread_id: @since_descendant_thread_id + 1)
   - @descendant_threads.each do |thread|
-    = render partial: 'stream_entries/status', collection: thread[:statuses], as: :status, locals: { is_successor: true, parent_id: status.id }
+    = render partial: 'stream_entries/status', collection: thread[:statuses], as: :status, locals: { is_successor: true, parent_id: status.id }, autoplay: autoplay
 
     - if thread[:next_status]
       .entry{ class: entry_classes }

diff --git a/app/views/stream_entries/embed.html.haml b/app/views/stream_entries/embed.html.haml
index d20c1e93e28c5e9ecd295a950659a5c7eae7c4be..4871c101e1cbf38e2f6fc574d8850fdd3fab787e 100644 (file)
--- a/app/views/stream_entries/embed.html.haml
+++ b/app/views/stream_entries/embed.html.haml
@@ -1,3 +1,3 @@
 - cache @stream_entry.activity do
   .activity-stream.activity-stream--headless
-    = render "stream_entries/#{@type}", @type.to_sym => @stream_entry.activity, centered: true
+    = render "stream_entries/#{@type}", @type.to_sym => @stream_entry.activity, centered: true, autoplay: @autoplay

diff --git a/config/brakeman.ignore b/config/brakeman.ignore
index 40fef7283f7eab7d585ecc670ab04ec85b14fed5..e5a5c16b4cb7f7d78aeb8e455727b7077907f4ec 100644 (file)
--- a/config/brakeman.ignore
+++ b/config/brakeman.ignore
@@ -84,7 +84,7 @@
       "check_name": "PermitAttributes",
       "message": "Potentially dangerous key allowed for mass assignment",
       "file": "app/controllers/admin/reports_controller.rb",
-      "line": 86,
+      "line": 80,
       "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
       "code": "params.permit(:account_id, :resolved, :target_account_id)",
       "render_path": null,
@@ -97,25 +97,6 @@
       "confidence": "High",
       "note": ""
     },
-    {
-      "warning_type": "Dynamic Render Path",
-      "warning_code": 15,
-      "fingerprint": "44d3f14e05d8fbb5b23e13ac02f15aa38b2a2f0f03b9ba76bab7f98e155a4a4e",
-      "check_name": "Render",
-      "message": "Render path contains parameter value",
-      "file": "app/views/stream_entries/embed.html.haml",
-      "line": 3,
-      "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
-      "code": "render(action => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :centered => true })",
-      "render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":58,"file":"app/controllers/statuses_controller.rb"}],
-      "location": {
-        "type": "template",
-        "template": "stream_entries/embed"
-      },
-      "user_input": "params[:id]",
-      "confidence": "Weak",
-      "note": ""
-    },
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
@@ -174,6 +155,25 @@
       "confidence": "Weak",
       "note": ""
     },
+    {
+      "warning_type": "Dynamic Render Path",
+      "warning_code": 15,
+      "fingerprint": "67afc0d5f7775fa5bd91d1912e1b5505aeedef61876347546fa20f92fd6915e6",
+      "check_name": "Render",
+      "message": "Render path contains parameter value",
+      "file": "app/views/stream_entries/embed.html.haml",
+      "line": 3,
+      "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
+      "code": "render(action => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :centered => true, :autoplay => ActiveModel::Type::Boolean.new.cast(params[:autoplay]) })",
+      "render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":59,"file":"app/controllers/statuses_controller.rb"}],
+      "location": {
+        "type": "template",
+        "template": "stream_entries/embed"
+      },
+      "user_input": "params[:id]",
+      "confidence": "Weak",
+      "note": ""
+    },
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 4,
@@ -388,6 +388,6 @@
       "note": ""
     }
   ],
-  "updated": "2018-08-18 00:49:25 +0200",
+  "updated": "2018-08-30 21:55:10 +0200",
   "brakeman_version": "4.2.1"
 }

diff --git a/spec/views/stream_entries/show.html.haml_spec.rb b/spec/views/stream_entries/show.html.haml_spec.rb
index 34207aa6befd18430cadcc4af5dad159907571ff..d06bb7abb5f33fd80b1ce7ec2c807dc3464d7787 100644 (file)
--- a/spec/views/stream_entries/show.html.haml_spec.rb
+++ b/spec/views/stream_entries/show.html.haml_spec.rb
@@ -12,6 +12,7 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d
     allow(view).to receive(:full_asset_url).and_return('//asset.host/image.svg')
     allow(view).to receive(:local_time)
     allow(view).to receive(:local_time_ago)
+    allow(view).to receive(:current_account).and_return(nil)
     assign(:instance_presenter, InstancePresenter.new)
   end