From 8f8e6776306b07d5914ef39c895de0cab44ada7f Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 12 Sep 2017 05:39:38 +0200 Subject: [PATCH] Clean up and improve generated OpenGraph tags (#4901) - Return all images as og:image - Return videos as og:image (preview) and og:video - Return profile:username on profiles --- app/helpers/application_helper.rb | 4 ++++ app/serializers/oembed_serializer.rb | 2 +- app/views/accounts/_og.html.haml | 17 +++++++------- app/views/accounts/show.html.haml | 2 +- .../stream_entries/_og_description.html.haml | 4 ++-- app/views/stream_entries/_og_image.html.haml | 22 +++++++++++++++---- app/views/stream_entries/show.html.haml | 10 ++++----- .../stream_entries/show.html.haml_spec.rb | 8 +++---- 8 files changed, 44 insertions(+), 25 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 61d4442c12b..6d625e7db9c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -42,4 +42,8 @@ module ApplicationHelper content_tag(:i, nil, attributes.merge(class: class_names.join(' '))) end + + def opengraph(property, content) + tag(:meta, content: content, property: property) + end end diff --git a/app/serializers/oembed_serializer.rb b/app/serializers/oembed_serializer.rb index af03fd47aca..0c8350e2d2b 100644 --- a/app/serializers/oembed_serializer.rb +++ b/app/serializers/oembed_serializer.rb @@ -45,7 +45,7 @@ class OEmbedSerializer < ActiveModel::Serializer height: height, } - content_tag(:iframe, nil, attributes) + content_tag(:script, nil, src: full_asset_url('embed.js'), async: true) + content_tag(:iframe, nil, attributes) + content_tag(:script, nil, src: full_asset_url('embed.js', skip_pipeline: true), async: true) end def width diff --git a/app/views/accounts/_og.html.haml b/app/views/accounts/_og.html.haml index 3ad39f391d7..1d16be590ab 100644 --- a/app/views/accounts/_og.html.haml +++ b/app/views/accounts/_og.html.haml @@ -1,8 +1,9 @@ -%meta{ property: 'og:url', content: url }/ -%meta{ property: 'og:site_name', content: site_title }/ -%meta{ property: 'og:title', content: [yield(:page_title).strip.presence, site_title].compact.join(' - ') }/ -%meta{ property: 'og:description', content: account.note }/ -%meta{ property: 'og:image', content: full_asset_url(account.avatar.url(:original)) }/ -%meta{ property: 'og:image:width', content: '120' }/ -%meta{ property: 'og:image:height', content: '120' }/ -%meta{ property: 'twitter:card', content: 'summary' }/ += opengraph 'og:url', url += opengraph 'og:site_name', site_title += opengraph 'og:title', [yield(:page_title).strip.presence, site_title].compact.join(' - ') += opengraph 'og:description', account.note += opengraph 'og:image', full_asset_url(account.avatar.url(:original)) += opengraph 'og:image:width', '120' += opengraph 'og:image:height', '120' += opengraph 'twitter:card', 'summary' += opengraph 'profile:username', account.local_username_and_domain diff --git a/app/views/accounts/show.html.haml b/app/views/accounts/show.html.haml index e0f9f869ace..6c90b2c04f8 100644 --- a/app/views/accounts/show.html.haml +++ b/app/views/accounts/show.html.haml @@ -9,7 +9,7 @@ %link{ rel: 'alternate', type: 'application/atom+xml', href: account_url(@account, format: 'atom') }/ %link{ rel: 'alternate', type: 'application/activity+json', href: ActivityPub::TagManager.instance.uri_for(@account) }/ - %meta{ property: 'og:type', content: 'profile' }/ + = opengraph 'og:type', 'profile' = render 'og', account: @account, url: short_account_url(@account, only_path: false) - if show_landing_strip? diff --git a/app/views/stream_entries/_og_description.html.haml b/app/views/stream_entries/_og_description.html.haml index 5762aca041c..d2fa99e63e5 100644 --- a/app/views/stream_entries/_og_description.html.haml +++ b/app/views/stream_entries/_og_description.html.haml @@ -1,4 +1,4 @@ - if activity.is_a?(Status) && activity.spoiler_text? - %meta{ property: 'og:description', content: activity.spoiler_text }/ + = opengraph 'og:description', activity.spoiler_text - else - %meta{ property: 'og:description', content: activity.content }/ + = opengraph 'og:description', activity.content diff --git a/app/views/stream_entries/_og_image.html.haml b/app/views/stream_entries/_og_image.html.haml index f725209d82a..66d9619b250 100644 --- a/app/views/stream_entries/_og_image.html.haml +++ b/app/views/stream_entries/_og_image.html.haml @@ -1,6 +1,20 @@ - if activity.is_a?(Status) && activity.non_sensitive_with_media? - %meta{ property: 'og:image', content: full_asset_url(activity.media_attachments.first.file.url(:small)) }/ + - activity.media_attachments.each do |media| + - if media.image? + = opengraph 'og:image', full_asset_url(media.file.url(:original)) + = opengraph 'og:image:type', media.file_content_type + = opengraph 'og:image:width', media.file.meta['original']['width'] + = opengraph 'og:image:height', media.file.meta['original']['height'] + - elsif media.video? + = opengraph 'og:image', full_asset_url(media.file.url(:small)) + = opengraph 'og:image:type', 'image/png' + = opengraph 'og:image:width', media.file.meta['small']['width'] + = opengraph 'og:image:height', media.file.meta['small']['height'] + = opengraph 'og:video', full_asset_url(media.file.url(:original)) + = opengraph 'og:video:type', media.file_content_type + = opengraph 'og:video:width', media.file.meta['small']['width'] + = opengraph 'og:video:height', media.file.meta['small']['height'] - else - %meta{ property: 'og:image', content: full_asset_url(account.avatar.url(:original)) }/ - %meta{ property: 'og:image:width', content: '120' }/ - %meta{ property: 'og:image:height', content: '120' }/ + = opengraph 'og:image', full_asset_url(account.avatar.url(:original)) + = opengraph 'og:image:width', '120' + = opengraph 'og:image:height','120' diff --git a/app/views/stream_entries/show.html.haml b/app/views/stream_entries/show.html.haml index 5ef72f80413..1bb8a32b251 100644 --- a/app/views/stream_entries/show.html.haml +++ b/app/views/stream_entries/show.html.haml @@ -6,15 +6,15 @@ %link{ rel: 'alternate', type: 'application/json+oembed', href: api_oembed_url(url: account_stream_entry_url(@account, @stream_entry), format: 'json') }/ %link{ rel: 'alternate', type: 'application/activity+json', href: ActivityPub::TagManager.instance.uri_for(@stream_entry.activity) }/ - %meta{ property: 'og:site_name', content: site_title }/ - %meta{ property: 'og:type', content: 'article' }/ - %meta{ property: 'og:title', content: "#{@account.username} on #{site_hostname}" }/ - %meta{ property: 'og:url', content: account_stream_entry_url(@account, @stream_entry) }/ + = opengraph 'og:site_name', site_title + = opengraph 'og:type', 'article' + = opengraph 'og:title', "#{@account.username} on #{site_hostname}" + = opengraph 'og:url', account_stream_entry_url(@account, @stream_entry) = render 'stream_entries/og_description', activity: @stream_entry.activity = render 'stream_entries/og_image', activity: @stream_entry.activity, account: @account - %meta{ property: 'twitter:card', content: 'summary' }/ + = opengraph 'twitter:card', 'summary_large_image' - if show_landing_strip? = render partial: 'shared/landing_strip', locals: { account: @stream_entry.account } diff --git a/spec/views/stream_entries/show.html.haml_spec.rb b/spec/views/stream_entries/show.html.haml_spec.rb index 6cc3b117aaa..59ea409907d 100644 --- a/spec/views/stream_entries/show.html.haml_spec.rb +++ b/spec/views/stream_entries/show.html.haml_spec.rb @@ -80,9 +80,9 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d header_tags = view.content_for(:header_tags) - expect(header_tags).to match(%r{}) - expect(header_tags).to match(%r{}) - expect(header_tags).to match(%r{}) - expect(header_tags).to match(%r{}) + expect(header_tags).to match(%r{}) + expect(header_tags).to match(%r{}) + expect(header_tags).to match(%r{}) + expect(header_tags).to match(%r{}) end end