From 11658d8653c7caadddfb0e3fe27272107c2e87b1 Mon Sep 17 00:00:00 2001 From: "Renato \"Lond\" Cerqueira" Date: Thu, 30 Aug 2018 23:14:01 +0200 Subject: [PATCH] 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 --- app/controllers/statuses_controller.rb | 1 + app/lib/formatter.rb | 20 +++++---- .../stream_entries/_detailed_status.html.haml | 11 +++-- .../stream_entries/_simple_status.html.haml | 11 +++-- app/views/stream_entries/_status.html.haml | 7 ++-- app/views/stream_entries/embed.html.haml | 2 +- config/brakeman.ignore | 42 +++++++++---------- .../stream_entries/show.html.haml_spec.rb | 1 + 8 files changed, 54 insertions(+), 41 deletions(-) diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 24824aabb..d4ad3df60 100644 --- 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 e1ab05cc0..0b4690394 100644 --- 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 7843005c1..6251cdd1a 100644 --- 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)}  %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 875795f2a..1d596d4e0 100644 --- 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)}  %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 92003a48f..83887cd87 100644 --- 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 d20c1e93e..4871c101e 100644 --- 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 40fef7283..e5a5c16b4 100644 --- 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 34207aa6b..d06bb7abb 100644 --- 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