Bug 52206 - deal with google changing caps node name
Summary: deal with google changing caps node name
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~jonny/te...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-07-17 15:32 UTC by Jonny Lamb
Modified: 2012-07-23 14:18 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Jonny Lamb 2012-07-17 15:32:19 UTC
#define GOOGLE_BUNDLE(cap, features) \
  gabble_presence_cache_add_bundle_caps (cache, \
      "http://www.google.com/xmpp/client/caps#" cap, features); \
  gabble_presence_cache_add_bundle_caps (cache, \
      "http://talk.google.com/xmpp/client/caps#" cap, features); \
  gabble_presence_cache_add_bundle_caps (cache, \
      "http://www.android.com/gtalk/client/caps#" cap, features); \
  gabble_presence_cache_add_bundle_caps (cache, \
      "http://www.android.com/gtalk/client/caps2#" cap, features); \
  gabble_presence_cache_add_bundle_caps (cache, \
      "http://talk.google.com/xmpp/bot/caps#" cap, features);

This kind of sucks because they've have five different node names. I've now found another.

If we've got recognisable google features in ext='…' then just use them regardless of what the node is. Here's a branch.
Comment 1 Simon McVittie 2012-07-17 18:03:20 UTC
(In reply to comment #0)
> If we've got recognisable google features in ext='…' then just use them
> regardless of what the node is. Here's a branch.

I'm kinda
Comment 2 Simon McVittie 2012-07-17 18:11:38 UTC
Let's try that again.

(In reply to comment #0)
> If we've got recognisable google features in ext='…' then just use them
> regardless of what the node is. Here's a branch.

I'm kinda suspicious about this because it violates the relevant XEP - but on the other hand, it's clearly what Google actually do (otherwise their various clients wouldn't interop with each other), so it's fine for anything that interoperates with Google. Also, 'ext' is a deprecated part of the XEP.

I vaguely wonder whether to do this but only for URLs matching \.(android|google)\.com/ but life is too short, and if we did, they'll probably start advertising google.co.uk caps just to spite us :-)

> + /* We also cache the ext='' bundles Gabble advertises: older Gabbles

There is no also, this is now the beginning of the function.

> + gabble_presence_set_capabilities (
> + presence, resource, cap_set, NULL, serial);

Er, if you have ext='voice-v1 video-v1' doesn't this end up setting voice-v1, then overwriting it with video-v1 so you lose the voice-v1 cap?
Comment 3 Simon McVittie 2012-07-17 18:15:23 UTC
(In reply to comment #2)
> I'm kinda suspicious about this because it violates the relevant XEP

... specifically XEP-0115 v1.3, "The names of the feature bundles MUST NOT be used for semantic purposes: they are merely opaque identifiers that will be used in other use cases". But Google clearly didn't read that sentence, so, whatever...
Comment 4 Jonny Lamb 2012-07-18 14:23:12 UTC
(In reply to comment #2)
> ...life is too short...

Amen to that.

> > + /* We also cache the ext='' bundles Gabble advertises: older Gabbles
> 
> There is no also, this is now the beginning of the function.

Oh yes, I removed the wrong word. Fixed.

> > + gabble_presence_set_capabilities (
> > + presence, resource, cap_set, NULL, serial);
> 
> Er, if you have ext='voice-v1 video-v1' doesn't this end up setting voice-v1,
> then overwriting it with video-v1 so you lose the voice-v1 cap?

No. I've updated the test to make it explicit that I'm testing this.

http://images.wikia.com/residentevil/images/8/84/Merchant_re4.jpg
GOT SOME RARE THINGS ON SALE, STRANGER.
Comment 5 Simon McVittie 2012-07-18 16:16:50 UTC
(In reply to comment #4)
> > > + gabble_presence_set_capabilities (
> > > + presence, resource, cap_set, NULL, serial);
> > 
> > Er, if you have ext='voice-v1 video-v1' doesn't this end up setting voice-v1,
> > then overwriting it with video-v1 so you lose the voice-v1 cap?
> 
> No. I've updated the test to make it explicit that I'm testing this.

After several false starts I realised this works because set_capabilities doesn't just mean "discard your caps, here are the new ones": it aggregates all calls to set_capabilities with the same "serial". I can't help thinking this ought to be simpler (asynchronously work out what the new set of caps-bundle URIs means, accumulate them in a set of capability URIs, then push them all into the presence in one go at the end?), but it's clearly not a merge blocker, so, ship it.
Comment 6 Jonny Lamb 2012-07-23 14:18:56 UTC
Merged.

   m    #                    #            
 mm#mm  # mm    mmm   m mm   #   m   mmm  
   #    #"  #  "   #  #"  #  # m"   #   " 
   #    #   #  m"""#  #   #  #"#     """m 
   "mm  #   #  "mm"#  #   #  #  "m  "mmm" 
                                          
                                          
   m""               
 mm#mm   mmm    m mm 
   #    #" "#   #"  "
   #    #   #   #    
   #    "#m#"   #    
                     
                     
   m    #            
 mm#mm  # mm    mmm  
   #    #"  #  #"  # 
   #    #   #  #"""" 
   "mm  #   #  "#mm" 
                     
                     
  m mm   mmm   m   m  mmm     mmm  m     m       
  #"  " #"  #  "m m"    #    #"  # "m m m"       
  #     #""""   #m#     #    #""""  #m#m#        
  #     "#mm"    #    mm#mm  "#mm"   # #     #   
                                            "    
                                                 
 #                 #      #               
 #mmm   m   m   mmm#   mmm#  m   m        
 #" "#  #   #  #" "#  #" "#  "m m"        
 #   #  #   #  #   #  #   #   #m#         
 ##m#"  "mm"#  "#m##  "#m##   "#      #   
                              m"          
                             ""


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.