From 72698bc3b49925a2b2955f32e5a562c1eecd729b Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 11 May 2017 00:28:10 +0200 Subject: [PATCH] Fix regressions from #2683 (#2970) * Fix regressions from #2683 Properly format spoiler text HTML, while keeping old logic for blankness intact Process hashtags and mentions in spoiler text Format spoiler text for Atom Change "show more" toggle into a button instead of anchor Fix style regression on dropdowns for detailed statuses * Fix lint issue * Convert spoiler text to plaintext in desktop notifications --- .../mastodon/actions/notifications.js | 2 +- .../mastodon/components/status_action_bar.js | 29 +++++++++------- .../mastodon/components/status_content.js | 21 +++++++----- .../features/status/components/action_bar.js | 5 ++- app/javascript/packs/public.js | 3 ++ app/javascript/styles/components.scss | 34 ++++++++++++++++++- app/lib/atom_serializer.rb | 2 +- app/lib/formatter.rb | 24 ++++--------- app/services/process_hashtags_service.rb | 2 +- app/services/process_mentions_service.rb | 4 ++- app/views/api/v1/statuses/_show.rabl | 3 +- .../stream_entries/_detailed_status.html.haml | 2 +- .../stream_entries/_simple_status.html.haml | 2 +- spec/lib/formatter_spec.rb | 4 +-- 14 files changed, 88 insertions(+), 49 deletions(-) diff --git a/app/javascript/mastodon/actions/notifications.js b/app/javascript/mastodon/actions/notifications.js index 7f8050330..61a245822 100644 --- a/app/javascript/mastodon/actions/notifications.js +++ b/app/javascript/mastodon/actions/notifications.js @@ -45,7 +45,7 @@ export function updateNotifications(notification, intlMessages, intlLocale) { // Desktop notifications if (typeof window.Notification !== 'undefined' && showAlert) { const title = new IntlMessageFormat(intlMessages[`notification.${notification.type}`], intlLocale).format({ name: notification.account.display_name.length > 0 ? notification.account.display_name : notification.account.username }); - const body = (notification.status && notification.status.spoiler_text.length > 0) ? notification.status.spoiler_text : unescapeHTML(notification.status ? notification.status.content : ''); + const body = (notification.status && notification.status.spoiler_text.length > 0) ? unescapeHTML(notification.status.spoiler_text) : unescapeHTML(notification.status ? notification.status.content : ''); new Notification(title, { body, icon: notification.account.avatar, tag: notification.id }); } diff --git a/app/javascript/mastodon/components/status_action_bar.js b/app/javascript/mastodon/components/status_action_bar.js index dc4466d6c..1c2445232 100644 --- a/app/javascript/mastodon/components/status_action_bar.js +++ b/app/javascript/mastodon/components/status_action_bar.js @@ -73,8 +73,12 @@ class StatusActionBar extends React.PureComponent { render () { const { status, me, intl } = this.props; - const reblog_disabled = status.get('visibility') === 'private' || status.get('visibility') === 'direct'; + const reblogDisabled = status.get('visibility') === 'private' || status.get('visibility') === 'direct'; + let menu = []; + let reblogIcon = 'retweet'; + let replyIcon; + let replyTitle; menu.push({ text: intl.formatMessage(messages.open), action: this.handleOpen }); menu.push(null); @@ -89,23 +93,24 @@ class StatusActionBar extends React.PureComponent { menu.push({ text: intl.formatMessage(messages.report, { name: status.getIn(['account', 'username']) }), action: this.handleReport }); } - let reblogIcon = 'retweet'; - if (status.get('visibility') === 'direct') reblogIcon = 'envelope'; - else if (status.get('visibility') === 'private') reblogIcon = 'lock'; - let reply_icon; - let reply_title; + if (status.get('visibility') === 'direct') { + reblogIcon = 'envelope'; + } else if (status.get('visibility') === 'private') { + reblogIcon = 'lock'; + } + if (status.get('in_reply_to_id', null) === null) { - reply_icon = "reply"; - reply_title = intl.formatMessage(messages.reply); + replyIcon = "reply"; + replyTitle = intl.formatMessage(messages.reply); } else { - reply_icon = "reply-all"; - reply_title = intl.formatMessage(messages.replyAll); + replyIcon = "reply-all"; + replyTitle = intl.formatMessage(messages.replyAll); } return (
-
-
+
+
diff --git a/app/javascript/mastodon/components/status_content.js b/app/javascript/mastodon/components/status_content.js index 1d462103b..e613f829f 100644 --- a/app/javascript/mastodon/components/status_content.js +++ b/app/javascript/mastodon/components/status_content.js @@ -11,9 +11,11 @@ class StatusContent extends React.PureComponent { constructor (props, context) { super(props, context); + this.state = { hidden: true }; + this.onMentionClick = this.onMentionClick.bind(this); this.onHashtagClick = this.onHashtagClick.bind(this); this.handleMouseDown = this.handleMouseDown.bind(this) @@ -36,8 +38,6 @@ class StatusContent extends React.PureComponent { link.setAttribute('title', mention.get('acct')); } else if (link.textContent[0] === '#' || (link.previousSibling && link.previousSibling.textContent && link.previousSibling.textContent[link.previousSibling.textContent.length - 1] === '#')) { link.addEventListener('click', this.onHashtagClick.bind(this, link.text), false); - } else if (media) { - link.innerHTML = ''; } else { link.setAttribute('target', '_blank'); link.setAttribute('rel', 'noopener'); @@ -70,11 +70,11 @@ class StatusContent extends React.PureComponent { const [ startX, startY ] = this.startXY; const [ deltaX, deltaY ] = [Math.abs(e.clientX - startX), Math.abs(e.clientY - startY)]; - if (e.target.localName === 'a' || (e.target.parentNode && e.target.parentNode.localName === 'a')) { + if (e.target.localName === 'button' || e.target.localName === 'a' || (e.target.parentNode && e.target.parentNode.localName === 'a')) { return; } - if (deltaX + deltaY < 5 && e.button === 0) { + if (deltaX + deltaY < 5 && e.button === 0 && this.props.onClick) { this.props.onClick(); } @@ -95,7 +95,7 @@ class StatusContent extends React.PureComponent { const { hidden } = this.state; const content = { __html: emojify(status.get('content')) }; - const spoilerContent = { __html: emojify(escapeTextContentForBrowser(status.get('spoiler_text', ''))) }; + const spoilerContent = { __html: emojify(status.get('spoiler_text', '')) }; const directionStyle = { direction: 'ltr' }; if (isRtl(status.get('content'))) { @@ -118,14 +118,19 @@ class StatusContent extends React.PureComponent { } return ( -
+
{mentionsPlaceholder} - ); } diff --git a/app/javascript/packs/public.js b/app/javascript/packs/public.js index 6aef2ffee..bbeb0a9ec 100644 --- a/app/javascript/packs/public.js +++ b/app/javascript/packs/public.js @@ -3,9 +3,12 @@ import { length } from 'stringz'; import { default as dateFormat } from 'date-fns/format'; import distanceInWordsStrict from 'date-fns/distance_in_words_strict'; import { delegate } from 'rails-ujs'; +import Rails from 'rails-ujs'; require.context('../images/', true); +Rails.start(); + const parseFormat = (format) => format.replace(/%(\w)/g, (_, modifier) => { switch (modifier) { case '%': diff --git a/app/javascript/styles/components.scss b/app/javascript/styles/components.scss index cededdcdd..3d133d4d9 100644 --- a/app/javascript/styles/components.scss +++ b/app/javascript/styles/components.scss @@ -474,15 +474,18 @@ } } -a.status__content__spoiler-link { +.status__content__spoiler-link { display: inline-block; border-radius: 2px; + background: transparent; + border: 0; color: lighten($ui-base-color, 8%); font-weight: 500; font-size: 11px; padding: 0 6px; text-transform: uppercase; line-height: inherit; + cursor: pointer; } .status__prepend-icon-wrapper { @@ -608,6 +611,34 @@ a.status__content__spoiler-link { width: 18px; } +.detailed-status__action-bar-dropdown { + flex: 1 1 auto; + display: flex; + align-items: center; + justify-content: center; + position: relative; + + .dropdown { + display: block; + width: 18px; + height: 18px; + } + + .dropdown--active { + .dropdown__content.dropdown__left { + left: 20px; + right: initial; + } + + &::after { + bottom: initial; + margin-left: 7px; + margin-top: -7px; + right: initial; + } + } +} + .detailed-status { background: lighten($ui-base-color, 4%); padding: 14px 10px; @@ -2165,6 +2196,7 @@ button.icon-button.active i.fa-retweet { display: flex; flex: 1 1 auto; align-items: center; + justify-content: center; a { color: $ui-highlight-color; diff --git a/app/lib/atom_serializer.rb b/app/lib/atom_serializer.rb index 920eac31b..6b0faf75f 100644 --- a/app/lib/atom_serializer.rb +++ b/app/lib/atom_serializer.rb @@ -332,7 +332,7 @@ class AtomSerializer end def serialize_status_attributes(entry, status) - append_element(entry, 'summary', status.spoiler_text, 'xml:lang': status.language) if status.spoiler_text? + append_element(entry, 'summary', Formatter.instance.format(status.proper, :spoiler_text, false).to_str, 'xml:lang': status.language, type: 'html') if status.spoiler_text? append_element(entry, 'content', Formatter.instance.format(status.proper).to_str, type: 'html', 'xml:lang': status.language) status.mentions.each do |mentioned| diff --git a/app/lib/formatter.rb b/app/lib/formatter.rb index 5b008278c..5a1b63c0a 100644 --- a/app/lib/formatter.rb +++ b/app/lib/formatter.rb @@ -9,13 +9,15 @@ class Formatter include ActionView::Helpers::TextHelper - def format(status) - return reformat(status.content) unless status.local? + def format(status, attribute = :text, paragraphize = true) + raw_content = status.public_send(attribute) - html = status.text + return '' if raw_content.blank? + return reformat(raw_content) unless status.local? + + html = raw_content html = encode_and_link_urls(html, status.mentions) - - html = simple_format(html, {}, sanitize: false) + html = simple_format(html, {}, sanitize: false) if paragraphize html = html.delete("\n") html.html_safe # rubocop:disable Rails/OutputSafety @@ -25,18 +27,6 @@ class Formatter sanitize(html, Sanitize::Config::MASTODON_STRICT).html_safe # rubocop:disable Rails/OutputSafety end - def format_spoiler(status) - return reformat(status.spoiler_text) unless status.local? - - html = status.spoiler_text - html = encode_and_link_urls(html) - - html = simple_format(html, {}, sanitize: false) - html = html.delete("\n") - - html.html_safe # rubocop:disable Rails/OutputSafety - end - def plaintext(status) return status.text if status.local? strip_tags(status.text) diff --git a/app/services/process_hashtags_service.rb b/app/services/process_hashtags_service.rb index eab033d50..8904e2cf8 100644 --- a/app/services/process_hashtags_service.rb +++ b/app/services/process_hashtags_service.rb @@ -2,7 +2,7 @@ class ProcessHashtagsService < BaseService def call(status, tags = []) - text = [status.text, status.spoiler_text].reject(&:empty?).join(' ') + text = [status.text, status.spoiler_text].reject(&:blank?).join(' ') tags = text.scan(Tag::HASHTAG_RE).map(&:first) if status.local? tags.map { |str| str.mb_chars.downcase }.uniq(&:to_s).each do |tag| diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index aa0a4d71b..fd45810fb 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -10,7 +10,9 @@ class ProcessMentionsService < BaseService def call(status) return unless status.local? - status.text.scan(Account::MENTION_RE).each do |match| + text = [status.text, status.spoiler_text].reject(&:blank?).join(' ') + + text.scan(Account::MENTION_RE).each do |match| username, domain = match.first.split('@') mentioned_account = Account.find_remote(username, domain) diff --git a/app/views/api/v1/statuses/_show.rabl b/app/views/api/v1/statuses/_show.rabl index 54e8a86d8..33bf39458 100644 --- a/app/views/api/v1/statuses/_show.rabl +++ b/app/views/api/v1/statuses/_show.rabl @@ -1,7 +1,8 @@ -attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id, :sensitive, :spoiler_text, :visibility +attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id, :sensitive, :visibility node(:uri) { |status| TagManager.instance.uri_for(status) } node(:content) { |status| Formatter.instance.format(status) } +node(:spoiler_text) { |status| Formatter.instance.format(status, :spoiler_text, false) } node(:url) { |status| TagManager.instance.url_for(status) } node(:reblogs_count) { |status| defined?(@reblogs_counts_map) ? (@reblogs_counts_map[status.id] || 0) : status.reblogs_count } node(:favourites_count) { |status| defined?(@favourites_counts_map) ? (@favourites_counts_map[status.id] || 0) : status.favourites_count } diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index 4cf94c83c..943d768dd 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -10,7 +10,7 @@ .status__content.p-name.emojify< - if status.spoiler_text? %p{ style: 'margin-bottom: 0' }< - %span.p-summary> #{Formatter.instance.format_spoiler(status)}  + %span.p-summary> #{Formatter.instance.format(status, :spoiler_text, false)}  %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.content) ? 'rtl' : 'ltr'}" }= Formatter.instance.format(status) diff --git a/app/views/stream_entries/_simple_status.html.haml b/app/views/stream_entries/_simple_status.html.haml index 583bb24f3..c07614295 100644 --- a/app/views/stream_entries/_simple_status.html.haml +++ b/app/views/stream_entries/_simple_status.html.haml @@ -16,7 +16,7 @@ .status__content.p-name.emojify< - if status.spoiler_text? %p{ style: 'margin-bottom: 0' }< - %span.p-summary> #{Formatter.instance.format_spoiler(status)}  + %span.p-summary> #{Formatter.instance.format(status, :spoiler_text, false)}  %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.content) ? 'rtl' : 'ltr'}" }= Formatter.instance.format(status) diff --git a/spec/lib/formatter_spec.rb b/spec/lib/formatter_spec.rb index 791bcce86..15b4e1d96 100644 --- a/spec/lib/formatter_spec.rb +++ b/spec/lib/formatter_spec.rb @@ -69,11 +69,9 @@ RSpec.describe Formatter do end end -=begin - it 'matches a URL without closing paranthesis' do + xit 'matches a URL without closing paranthesis' do expect(subject.match('(http://google.com/)')[0]).to eq 'http://google.com' end -=end context 'matches a URL without exclamation point' do let(:local_text) { 'http://www.google.com!' }