Fix up the meaning of the name parameter in mentions

The desktop frontend now treats the "name" parameter of mention as
a string to display unconditionally. But the Diaspora::Mentionable
renders mentions the different way: "name" is treated as a fallback
string which is rendered only if the person's name is unavailable.
This reflects on the mobile version ATM. This patch makes it behave
the same way as the current desktop version does.

app/helpers/people_helper.rb

@@ -28,7 +28,9 @@ module PeopleHelper
     opts[:class] << " self" if defined?(user_signed_in?) && user_signed_in? && current_user.person == person
     opts[:class] << " hovercardable" if defined?(user_signed_in?) && user_signed_in? && current_user.person != person
     remote_or_hovercard_link = Rails.application.routes.url_helpers.person_path(person).html_safe
-    "<a data-hovercard='#{remote_or_hovercard_link}' href='#{remote_or_hovercard_link}' class='#{opts[:class]}' #{ ("target=" + opts[:target]) if opts[:target]}>#{h(person.name)}</a>".html_safe
+    "<a data-hovercard='#{remote_or_hovercard_link}' href='#{remote_or_hovercard_link}' class='#{opts[:class]}'>"\
+      "#{html_escape_once(opts[:display_name] || person.name)}</a>"\
+      .html_safe
   end
 
   def person_image_tag(person, size = :thumb_small)
@@ -58,7 +60,7 @@ module PeopleHelper
     absolute = opts.delete(:absolute)
 
     if person.local?
-      username = person.diaspora_handle.split('@')[0]
+      username = person.username
       unless username.include?('.')
         opts.merge!(:username => username)
         if absolute

lib/diaspora/mentionable.rb

@@ -14,8 +14,8 @@ module Diaspora::Mentionable
     mention = mention_str.match(REGEX)[2]
     del_pos = mention.rindex(/;/)
 
-    name   = mention[0..(del_pos-1)].strip
+    name = mention[0..(del_pos - 1)].strip
-    handle = mention[(del_pos+1)..-1].strip
+    handle = mention[(del_pos + 1)..-1].strip
 
     [name, handle]
   end
@@ -85,33 +85,33 @@ module Diaspora::Mentionable
   module MentionsInternal
     extend ::PeopleHelper
 
-    # output a formatted mention link as defined by the given options,
+    # output a formatted mention link as defined by the given arguments.
-    # use the fallback name if the person is unavailable
+    # if the display name is blank, falls back to the person's name.
     # @see Diaspora::Mentions#format
     #
     # @param [Person] AR Person
-    # @param [String] fallback name
+    # @param [String] display name
     # @param [Hash] formatting options
-    def self.mention_link(person, fallback_name, opts)
+    def self.mention_link(person, display_name, opts)
-      return fallback_name unless person.present?
+      return display_name unless person.present?
 
       if opts[:plain_text]
-        person.name
+        display_name.presence || person.name
       else
-        person_link(person, class: PERSON_HREF_CLASS)
+        person_link(person, class: PERSON_HREF_CLASS, display_name: display_name)
       end
     end
 
-    # output a markdown formatted link to the given person or the given fallback
+    # output a markdown formatted link to the given person with the display name as the link text.
-    # string, in case the person is not present
+    # if the display name is blank, falls back to the person's name.
     #
     # @param [Person] AR Person
-    # @param [String] fallback name
+    # @param [String] display name
     # @return [String] markdown person link
-    def self.profile_link(person, fallback_name)
+    def self.profile_link(person, display_name)
-      return fallback_name unless person.present?
+      return display_name unless person.present?
 
-      "[#{person.name}](#{local_or_remote_person_path(person)})"
+      "[#{display_name.presence || person.name}](#{local_or_remote_person_path(person)})"
     end
 
     # takes a user and an array of aspect ids or an array containing "all" as

spec/helpers/people_helper_spec.rb

@@ -77,6 +77,11 @@ describe PeopleHelper, :type => :helper do
     it 'links by id for a local user' do
       expect(person_link(@user.person)).to include "href='#{person_path(@user.person)}'"
     end
+
+    it "recognizes the 'display_name' option" do
+      display_name = "string used as a name"
+      expect(person_link(@person, display_name: display_name)).to match(%r{<a [^>]+>#{display_name}</a>})
+    end
   end
 
   describe '#local_or_remote_person_path' do

spec/integration/mentioning_spec.rb

@@ -52,6 +52,7 @@ describe "mentioning", type: :request do
     expect(status_msg.public?).to be false
     expect(status_msg.text).to include(@user3.name)
     expect(status_msg.text).not_to include(@user3.diaspora_handle)
+    expect(status_msg.text).to include(user_profile_path(username: @user3.username))
 
     expect(stream_for(@user3).map(&:id)).not_to include(status_msg.id)
     expect(mention_stream_for(@user3).map(&:id)).not_to include(status_msg.id)

spec/lib/diaspora/mentionable_spec.rb

@@ -6,11 +6,12 @@ describe Diaspora::Mentionable do
 
   before do
     @people = [alice, bob, eve].map(&:person)
+    @names = %w(Alice\ A Bob\ B "Eve>\ E)
     @test_txt = <<-STR
 This post contains a lot of mentions
-one @{Alice A; #{@people[0].diaspora_handle}},
+one @{#{@names[0]}; #{@people[0].diaspora_handle}},
-two @{Bob B; #{@people[1].diaspora_handle}} and finally
+two @{#{@names[1]}; #{@people[1].diaspora_handle}} and finally
-three @{"Eve> E; #{@people[2].diaspora_handle}}.
+three @{#{@names[2]}; #{@people[2].diaspora_handle}}.
 STR
     @test_txt_plain = <<-STR
 This post contains a lot of mentions
@@ -18,53 +19,50 @@ one Alice A,
 two Bob B and finally
 three &quot;Eve&gt; E.
 STR
-    @status_msg = FactoryGirl.build(:status_message, text: @test_txt)
   end
 
   describe "#format" do
     context "html output" do
       it "adds the links to the formatted message" do
-        fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people)
+        fmt_msg = Diaspora::Mentionable.format(@test_txt, @people)
 
-        @people.each do |person|
+        [@people, @names].transpose.each do |person, name|
-          expect(fmt_msg).to include person_link(person, class: "mention hovercardable")
+          expect(fmt_msg).to include person_link(person, class: "mention hovercardable", display_name: name)
         end
       end
 
       it "should work correct when message is escaped html" do
-        raw_msg = @status_msg.text
+        fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(@test_txt), @people)
-        fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(raw_msg), @people)
 
-        @people.each do |person|
+        [@people, @names].transpose.each do |person, name|
-          expect(fmt_msg).to include person_link(person, class: "mention hovercardable")
+          expect(fmt_msg).to include person_link(person, class: "mention hovercardable", display_name: name)
         end
       end
 
       it "escapes the link title (name)" do
-        p = @people[0].profile
+        name = "</a><script>alert('h')</script>"
-        p.first_name = "</a><script>alert('h')</script>"
+        test_txt = "two @{#{name}; #{@people[0].diaspora_handle}} and finally"
-        p.save!
 
-        fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people)
+        fmt_msg = Diaspora::Mentionable.format(test_txt, @people)
 
-        expect(fmt_msg).not_to include(p.first_name)
+        expect(fmt_msg).not_to include(name)
         expect(fmt_msg).to include("&gt;", "&lt;", "&#39;") # ">", "<", "'"
       end
     end
 
     context "plain text output" do
       it "removes mention markup and displays unformatted name" do
-        fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people, plain_text: true)
+        fmt_msg = Diaspora::Mentionable.format(@test_txt, @people, plain_text: true)
 
-        @people.each do |person|
+        @names.each do |name|
-          expect(fmt_msg).to include person.first_name
+          expect(fmt_msg).to include CGI.escapeHTML(name)
         end
         expect(fmt_msg).not_to include "<a", "</a>", "hovercardable"
       end
     end
 
-    it "leaves the name of people that cannot be found" do
+    it "leaves the names of people that cannot be found" do
-      fmt_msg = Diaspora::Mentionable.format(@status_msg.text, [])
+      fmt_msg = Diaspora::Mentionable.format(@test_txt, [])
       expect(fmt_msg).to eql @test_txt_plain
     end
   end
@@ -111,7 +109,7 @@ STR
       aspect_id = @user_a.aspects.where(name: "generic").first.id
       txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_c, @user_a, aspect_id)
 
-      expect(txt).to include(@user_c.person.name)
+      expect(txt).to include("user C")
       expect(txt).to include(local_or_remote_person_path(@user_c.person))
       expect(txt).not_to include("href")
       expect(txt).not_to include(@mention_c)