Bug 98127 - Make disco#info advertise disco#info feature
Summary: Make disco#info advertise disco#info feature
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: All All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-06 20:53 UTC by Maxime Buquet
Modified: 2016-10-12 18:54 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Add disco NS to the capability list (701 bytes, patch)
2016-10-06 20:53 UTC, Maxime Buquet
Details | Splinter Review
signature.asc (818 bytes, application/pgp-signature)
2016-10-12 18:54 UTC, Maxime Buquet
Details

Description Maxime Buquet 2016-10-06 20:53:57 UTC
Created attachment 127081 [details] [review]
Add disco NS to the capability list

XEP-0030 §3.1 “Note: Every entity MUST […] support at least the 'http://jabber.org/protocol/disco#info' feature”

And with a patch attached!
Comment 1 diane 2016-10-06 21:11:34 UTC
two things shouldn't the URL be:

 https://xmpp.org/extensions/xep-0030.html#info-basic

And a different question is, MUST every entity advertise that it supports disco#info?

I used telepathy-gabble-xmpp-console and checked some of my XMPP clients.

Debian's gabble 0.18.3-2+b1 doesn't advertise disco#info, Gajim 0.16.5-2 and Conversations 1.14 something (not sure which device I browsed) does report disco#info support.

Did you check if it disco#info shows up after applying the patch?
Comment 2 Maxime Buquet 2016-10-07 00:22:14 UTC
On 2016/10/06, bugzilla-daemon@freedesktop.org wrote:
> https://bugs.freedesktop.org/show_bug.cgi?id=98127
> 
> --- Comment #1 from diane@ghic.org ---
> two things shouldn't the URL be:
> 
>  https://xmpp.org/extensions/xep-0030.html#info-basic

No, namespace URIs don't use XEP addresses, take a look at the first
paragraph after Example 1

> Debian's gabble 0.18.3-2+b1 doesn't advertise disco#info, Gajim 0.16.5-2 and
> Conversations 1.14 something (not sure which device I browsed) does report
> disco#info support.
> 
> Did you check if it disco#info shows up after applying the patch?

Debian's gabble doesn't advertise it, that's why I'm sending a patch!
Comment 3 diane 2016-10-10 17:42:17 UTC
Oops I got confused about the URL I thought you were linking to the documentation not the namespace.
Comment 4 diane 2016-10-10 18:39:35 UTC
+1 looks good to me.

My testing was:
 * add it as a patch to the Debian package of 0.18.3 and verifying that indeed a disco#info request did report that gabble supported disco#info
 * Apply it to my proposed-updates branch with several unit test fixes and run the unit tests.

I had one test failure tls/server-tls-channel.py regardless of whether or not this patch was applied.

I asked on the xmpp jdev MUC if including disco#info was required and got back this response.

"yes, it’s explicitly required in XEP-0030, but I remember some clients forgot to do so so you shouldn’t consider its absence as a sign the client doesn’t support it."

So what branch should we be submitting code to?
Comment 5 George Kiagiadakis 2016-10-12 16:40:21 UTC
(In reply to diane from comment #4)
> "yes, it’s explicitly required in XEP-0030, but I remember some clients
> forgot to do so so you shouldn’t consider its absence as a sign the client
> doesn’t support it."

Funny that the rest of the examples on the XEP do not include it...

> So what branch should we be submitting code to?

The stable branch, telepathy-gabble-0.18

I have committed it now.
Comment 6 Maxime Buquet 2016-10-12 18:54:02 UTC
Created attachment 127254 [details]
signature.asc

> > So what branch should we be submitting code to?
> 
> The stable branch, telepathy-gabble-0.18
> 
> I have committed it now.

Thanks!


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.