Bug 24699

Summary: don't abuse M_T_Action for nudges
Product: Telepathy Reporter: Simon McVittie <smcv>
Component: butterflyAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: olivier
Version: unspecifiedKeywords: patch
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/jonny/telepathy-butterfly.git;a=shortlog;h=refs/heads/nudges
Whiteboard: review+
i915 platform: i915 features:

Description Simon McVittie 2009-10-23 08:14:57 UTC
16:06 < smcv> does MSN have the concept of an action message?
16:06 < smcv> arguably Empathy should turn (ACTION, "drinks heavily") into 
              (NORMAL, "*** drinks heavily") for protocols that can't do 
              actions properly
16:06 < istaz> wjt: well it only support if it fhe action is nudge
16:07 < smcv> istaz: \o\ /o/ \o/ MSN!
16:07 < istaz> we could probably convert it inside butterfly to display a irc 
               style message
16:07 < wjt> Maybe Empathy should send "/me laughs" as-is
16:07 < smcv> istaz: hang on. so you claim to support the ACTION message type, 
              then if the action is not "nudge", you error?
16:07 < wjt> (if the CM doesn't do Message_Type_Action)
16:08 < istaz> smcv: yep
16:08 < wjt> !!
16:08 < wjt> that's not what M_T_Action is for
16:08 < smcv> istaz: please, no
16:08 < cassidy> istaz, sounds great
16:08 < smcv> istaz: that blocks the UI's ability to do what I suggested a 
              dozen lines ago, or what wjt suggested half a dozen lines ago
16:08 < smcv> having the UI be
16:08 < smcv> *** drinks heavily
16:09 < smcv> or even
16:09 < smcv> /me drinks heavily
16:09 < smcv> would be considerably better than just erroring!
16:09 < smcv> if we support nudges, it should be via an interface that can also               support http://xmpp.org/extensions/xep-0224.html
16:09 < smcv> probably
16:09 < istaz> smcv: couldn't the ui do it when it receive an error?
16:10 < smcv> istaz: I would consider that to be deeply, deeply bong.
16:10 < smcv> istaz: "InvalidArgument" (or whatever it is you reply with) is 
              nowhere near enough information to infer "oh, this Channel that 
              claims to support ACTION secretly doesn't"
16:11 < istaz> smcv: and another ugly things is that when we receive a nudge we 
               actually send a message "<contact> sends you a nudge" (without 
               localization)
16:11 < smcv> istaz: that's wrong too, but a little less so
16:11 < istaz> smcv: I should probably fix that then
16:11 < Zdra> smcv, empathy does not uses Messages
16:11 < Zdra> smcv, and does not try any caps
16:12 < Zdra> smcv, we are using old Text iface ;)
16:12 < smcv> Zdra: ok, so Empathy is also at fault, it should call 
              Text.GetMessageTypes(), and use the result for graceful fallback
16:12 < smcv> Zdra: however, butterfly currently prevents that from working in 
              any case
16:13 < smcv> to the bug-filing-mobile!
Comment 1 Simon McVittie 2009-10-23 08:19:43 UTC
This prevents https://bugzilla.gnome.org/show_bug.cgi?id=599415 from being solved for Butterfly Text channels.
Comment 2 Simon McVittie 2009-10-23 08:25:00 UTC
Retitling; removing the secret nudge pseudo-API isn't strictly a requirement to have Empathy work, although I still think it's wrong.
Comment 3 Jonny Lamb 2010-03-15 09:50:45 UTC
We already do this:

method call sender=:1.48 -> dest=:1.28 serial=131 path=/org/freedesktop/Telepathy/Connection/butterfly/msn/jonny_40jonnylamb_2ecom/TextChannel1; interface=org.freedesktop.Telepathy.Channel.Type.Text; member=GetMessageTypes
method return sender=:1.28 -> dest=:1.48 reply_serial=131
   array [
      uint32 0
   ]


16:42 < jonnylamb> smcv: I'm confused as to what you're actually asking 
                   for in fd.o#24699 now.
16:42 < jonnylamb> smcv: You note in the Empathy bug that butterfly only 
                   returns NORMAL in GetMessageTypes().
16:42 < jonnylamb> So surely "should not advertise Message_Type_Action 
                   unless able to deal with all messages of that type" is 
                   satisfied?
16:43 < smcv> jonnylamb: if that is the case then yes
16:44 < smcv> jonnylamb: I was wrong about the butterfly bug initially, I 
              thought butterfly advertised Normal and Action but Action 
              only worked for nudges
16:44 < jonnylamb> You're right about the second part -- if we're given 
                   an action message type and it's a nudge then we send a 
                   nudge over the wire, otherwise we, don't?
16:45 < smcv> (Normal, *) -> "*"
16:45 < istaz> jonnylamb: it was about the /nudge, we only return NORMAL 
               but empathy don't check that, and /nudge is only accepted 
               as ACTION_TYPE and don't accept if that's not /nudge
16:45 < jonnylamb> But seeing as we don't advertise ACTION, I think we 
                   can do what the hell we want if someone gives us an 
                   Action message. :-)
16:45 < smcv> (Action, "nudge") -> protocol nudge
16:45 < smcv> (Action, "dies") -> error
16:45 < smcv> jonnylamb: in principle you're right, although I still 
              think it's an abuse of Action messages
16:46 < smcv> and the Empathy bug isn't blocked by the butterfly bug as I 
              initially thought, but is still a rather annoying bug
16:46 < jonnylamb> I'm actually in favour of removing nudge support in 
                   butterfly.
16:46 < smcv> s/in butterfly// :-)
16:46 < jonnylamb> Maybe we should hook it up so if a contact nudges you, 
                   he or she is automatically blocked.
16:48 < jonnylamb> Anyway, I'm going to close that bug because it's not a 
                   bug. If you want the bug to mean something else, say 
                   now or forever hold your peace.
16:48 < smcv> indeed, nudging should either be ignored, or be a 
              Channel.Interface.Annoyance or something (which Empathy 
              would implement as the URGENT hint
16:48 < smcv> )
16:49 < smcv> jonnylamb: would you mind re-purposing it as "don't abuse 
              M_T_Action for nudges"? (if so you can drop the severity of 
              course)
16:49 < jonnylamb> ok
Comment 4 Jonny Lamb 2010-03-15 10:01:44 UTC
Check out my patch.
Comment 5 Simon McVittie 2010-03-15 10:04:27 UTC
review+
Comment 6 Jonny Lamb 2010-03-15 10:14:05 UTC
Thanks.
Comment 7 Olivier Le Thanh Duong 2010-03-16 05:20:39 UTC
This patch remove the ability to nudge completly, should'nt we accept /nudge in text channel instead?

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.