Closed Bug 955130 Opened 10 years ago Closed 10 years ago

Add VKontakte protocol

Categories

(Chat Core :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: unghost)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [mentor=clokep])

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 1702 by Alexander L. Slovesnik <unghost AT gmail.com> at 2012-09-21 18:28:00 UTC ***

Currently there is not possible for localizers to ship additional Jabber networks with their localizations. However, there are a lot of Jabber networks, popular in specific countries, and some of them can compete with Gtalk or Facebook. For example, popular Russian social networks - VKontakte ( http://en.wikipedia.org/wiki/VK_%28social_network%29 ) and Odnoklassniki ( http://en.wikipedia.org/wiki/Odnoklassniki ) are more popular than Facebook in Russia. And both of them have Jabber services.
In order to add Jabber services for these social networks, users have to jump through several hoops (find help pages with XMPP settings, then figure out how to apply them to Instantbird). I propose to add the ability to ship Jabber networks with Instantbird as plugins, similar to web-search plugins in Firefox/Thunderbird, so localizers could ship popular Jabber networks with their localizations.
Another option would be implement something similar to ISPDB database in Thunderbird, so user could enter only username, password and domain and Instantbird would fetch all settings for this Jabber network from database on Instantbird server, but it looks to me as overkill.
*** Original post on bio 1702 at 2012-09-22 03:57:49 UTC ***

(In reply to comment #0)
> In order to add Jabber services for these social networks, users have to jump
> through several hoops (find help pages with XMPP settings, then figure out how
> to apply them to Instantbird).
I wouldn't call it jumping through hoops. ;) But they do need to configure it.

> I propose to add the ability to ship Jabber
> networks with Instantbird as plugins, similar to web-search plugins in
> Firefox/Thunderbird,
We essentially do this already with Facebook and Google Talk, is what you're suggesting any different? (See http://lxr.instantbird.org/instantbird/source/chat/protocols/gtalk/gtalk.js, for example.)

> so localizers could ship popular Jabber networks with
> their localizations.
If we want to add these, I think we should just add them in general and whatever languages they are appropriate for can add them to the top protocols list. I think it will be confusing to have totally different networks listed in different localized versions.

> Another option would be implement something similar to ISPDB database in
> Thunderbird, so user could enter only username, password and domain and
> Instantbird would fetch all settings for this Jabber network from database on
> Instantbird server, but it looks to me as overkill.
I agree that this seems a bit excessive, but it would certainly be useful for XMPP and IRC. I'm not sure if entering just a username / domain would work or if it should be selected from a list, etc.
Component: Other → General
Product: Instantbird → Chat Core
*** Original post on bio 1702 at 2012-10-15 18:50:20 UTC ***

(In reply to comment #1)
> > so localizers could ship popular Jabber networks with
> > their localizations.
> If we want to add these, I think we should just add them in general and
> whatever languages they are appropriate for can add them to the top protocols
> list. I think it will be confusing to have totally different networks listed in
> different localized versions.

Just to follow up on this: would you be interested in adding VKontakte and/or Odnoklassniki to Instantbird as protocols? As clokep suggests, you'd only have to adapt what was done for Facebook (http://lxr.instantbird.org/instantbird/source/chat/protocols/facebook/facebook.js), gtalk (http://lxr.instantbird.org/instantbird/source/chat/protocols/gtalk/gtalk.js), or VZnet (https://addons.instantbird.org/en-US/instantbird/addon/254). There is probably very little actual coding involved as if they are just XMPP servers, it's essentially a matter of "filling in the XMPP property fields by default". The #instantbird channel is a good place to come if you'd like to discuss this or have questions...
*** Original post on bio 1702 at 2012-10-15 18:55:02 UTC ***

I'm available via IRC or email to help with this, if you'd like. (It's possible I could even do it if given all the information...someone else would need to test it for me though. :))
Whiteboard: [mentor=clokep]
Attached patch Add VKontakte protocol v.0.1 (obsolete) — Splinter Review
*** Original post on bio 1702 as attmnt 1970 by unghost AT gmail.com at 2012-10-16 18:54:00 UTC ***

I've tried to make patch for VKontakte. Unfortunately, I have not managed to compile Instantird with this patch applied. My build on Ubuntu 12.10 fails while compiling libpurple. It probably doesn't work.
Attachment #8353729 - Flags: feedback?(clokep)
*** Original post on bio 1702 by Alexander L. Slovesnik <unghost AT gmail.com> at 2012-10-16 19:16:35 UTC ***

(In reply to comment #3)
> I'm available via IRC or email to help with this, if you'd like. (It's possible
> I could even do it if given all the information...someone else would need to
> test it for me though. :))

I've mailed you login/password for test account in VKontakte.
Comment on attachment 8353729 [details] [diff] [review]
Add VKontakte protocol v.0.1

*** Original change on bio 1702 attmnt 1970 at 2012-10-17 00:01:48 UTC ***

This is a great start! I'll make a few comments, please let me know if it isn't clear!

>diff --git a/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png b/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png
Where are these images from? Can we use them?

>diff --git a/chat/protocols/vkontakte/vkontakte.js b/chat/protocols/vkontakte/vkontakte.js
>+XPCOMUtils.defineLazyGetter(this, "_", function()
>+  l10nHelper("chrome://chat/locale/xmpp.properties")
>+);
>+
>+
Please only one empty line here.

>+function VkontakteAccount(aProtoInstance, aImAccount) {
>+  this._init(aProtoInstance, aImAccount);
>+}
>+VkontakteAccount.prototype = {
>+  __proto__: XMPPAccountPrototype,
>+  connect: function() {
>+    this._jid = this._parseJID(this.name);
>+    // The XMPP spec says that the node part of a JID is optional, but
>+    // in the case of Vkontakte if the username typed by the user
>+    // doesn't contain an @, we prefer assuming that it's the domain
>+    // part that's been omitted.
>+    if (!this._jid.node) {
>+      // If the domain part was omitted, swap the node and domain parts,
>+      // use 'gmail.com' as the default domain, and tell the Google
>+      // Talk server that we will use the full bind result.
>+      this._jid.node = this._jid.domain;
>+      this._jid.domain = "vk.com";
>+      this.authMechanisms = {PLAIN: PlainFullBindAuth};
>+    }
This refers to a specific GTalk extension here, you probably want to look more at the Facebook protocol here: http://lxr.instantbird.org/instantbird/source/chat/protocols/facebook/facebook.js#22 (for the whole connect method).

>+function VkontakteProtocol() {
[...]
>+  classID: Components.ID("{0743ab81-8963-1743-7abc-9874823acd56}")
Glad to see you updated the UUID! :)

>diff --git a/instantbird/content/account.js b/instantbird/content/account.js
>     let protoId = this.proto.id;
>     if (protoId == "prpl-irc" || protoId == "prpl-jabber" ||
>-        protoId == "prpl-gtalk") {
>+        protoId == "prpl-gtalk" ||
>+        protoId == "prpl-vkontakte") {
I just want to confirm to Vkontakte supports chatrooms, is that true?

(As a side note...I really wish we didn't have to do this.)

>diff --git a/instantbird/locales/en-US/chrome/instantbird/accountWizard.properties b/instantbird/locales/en-US/chrome/instantbird/accountWizard.properties
[...]
>+topProtocol.prpl-vkontakte.description=Communicate with friends in VKontakte
I don't think we want this change for en-US, but it should definitely be included for whatever languages use Vkontakte...although I guess there isn't really an issue with include the description in the en-US version, as long as it isn't in the topProtocol.list. I'll defer to Florian here.
Attachment #8353729 - Flags: feedback?(clokep) → feedback+
*** Original post on bio 1702 by Alexander L. Slovesnik <unghost AT gmail.com> at 2012-10-17 10:48:40 UTC ***

(In reply to comment #6)
> Comment on attachment 8353729 [details] [diff] [review] (bio-attmnt 1970) [details]
> Add VKontakte protocol v.0.1
> >diff --git a/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png b/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png
> Where are these images from? Can we use them?
From http://blog.ragnaar.com/2009/05/ikonki-dlya-vkontakte.html
Do I need to ask permission to use them?
 
> >diff --git a/instantbird/content/account.js b/instantbird/content/account.js
> >     let protoId = this.proto.id;
> >     if (protoId == "prpl-irc" || protoId == "prpl-jabber" ||
> >-        protoId == "prpl-gtalk") {
> >+        protoId == "prpl-gtalk" ||
> >+        protoId == "prpl-vkontakte") {
> I just want to confirm to Vkontakte supports chatrooms, is that true?
No. I don't think that Vkontakte supports chatrooms. Should I undo this change? 

 
> >diff --git a/instantbird/locales/en-US/chrome/instantbird/accountWizard.properties b/instantbird/locales/en-US/chrome/instantbird/accountWizard.properties
> [...]
> >+topProtocol.prpl-vkontakte.description=Communicate with friends in VKontakte
> I don't think we want this change for en-US, but it should definitely be
> included for whatever languages use Vkontakte...although I guess there isn't
> really an issue with include the description in the en-US version, as long as
> it isn't in the topProtocol.list. I'll defer to Florian here.

Vkontakte is popular in ex-USSR countries, so ex-USSR locales may be insterested. vk.com is #3 in Ukraine ( http://www.alexa.com/topsites/countries/UA ) and #9 in Estonia ( http://www.alexa.com/topsites/countries/EE ).
*** Original post on bio 1702 at 2012-10-17 11:43:33 UTC ***

Confirming and renaming this bug (we can open a new one for Odnoklassniki later).
Assignee: nobody → bugzilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: RFE: Allow localizers to ship additional Jabber networks with their localizations → Add VKontakte protocol
*** Original post on bio 1702 at 2012-10-17 12:06:12 UTC ***

(In reply to comment #7)
> (In reply to comment #6)
> > Comment on attachment 8353729 [details] [diff] [review] (bio-attmnt 1970) [details]
> > Add VKontakte protocol v.0.1
> > >diff --git a/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png b/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png
> > Where are these images from? Can we use them?
> From http://blog.ragnaar.com/2009/05/ikonki-dlya-vkontakte.html
> Do I need to ask permission to use them?
It looks like it is implied they're free to use, but I don't read Russian (?) and there's no explicit license on that page. I think it would be best to ask (unless Florian has an opinion...I don't really know how media is licensed).

> > >diff --git a/instantbird/content/account.js b/instantbird/content/account.js
> > >     let protoId = this.proto.id;
> > >     if (protoId == "prpl-irc" || protoId == "prpl-jabber" ||
> > >-        protoId == "prpl-gtalk") {
> > >+        protoId == "prpl-gtalk" ||
> > >+        protoId == "prpl-vkontakte") {
> > I just want to confirm to Vkontakte supports chatrooms, is that true?
> No. I don't think that Vkontakte supports chatrooms. Should I undo this change?
Yes, these changes have to do with joining chats. (That's why gtalk is listed, but not Facebook!)

> > >diff --git a/instantbird/locales/en-US/chrome/instantbird
> Vkontakte is popular in ex-USSR countries, so ex-USSR locales may be
> insterested. vk.com is #3 in Ukraine (
> http://www.alexa.com/topsites/countries/UA ) and #9 in Estonia (
> http://www.alexa.com/topsites/countries/EE ).
So for those languages we'd definitely want to add prpl-vkontakte to topProtocol.list! :) Thanks for the information.

Great job so far, I think a couple more iterations and it'll be ready!
*** Original post on bio 1702 at 2012-10-17 12:22:01 UTC ***

(In reply to comment #9)

> > > >diff --git a/instantbird/locales/en-US/chrome/instantbird
> > Vkontakte is popular in ex-USSR countries, so ex-USSR locales may be
> > insterested. vk.com is #3 in Ukraine (
> > http://www.alexa.com/topsites/countries/UA ) and #9 in Estonia (
> > http://www.alexa.com/topsites/countries/EE ).
> So for those languages we'd definitely want to add prpl-vkontakte to
> topProtocol.list! :) Thanks for the information.

You mean that localizers for these locales will want to add a prpl-vkontakte string in their localized accountWizard.properties, not that we need it in the en-US file, right?
*** Original post on bio 1702 at 2012-10-17 12:26:56 UTC ***

(In reply to comment #10)
> (In reply to comment #9)
> > So for those languages we'd definitely want to add prpl-vkontakte to
> > topProtocol.list! :) Thanks for the information.
> 
> You mean that localizers for these locales will want to add a prpl-vkontakte
> string in their localized accountWizard.properties, not that we need it in the
> en-US file, right?

Yes, sorry if that wasn't clear.
Status: NEW → ASSIGNED
Attached patch Add VKontakte protocol v.0.2 (obsolete) — Splinter Review
*** Original post on bio 1702 as attmnt 1979 by unghost AT gmail.com at 2012-10-17 19:58:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353738 - Flags: review?(clokep)
Comment on attachment 8353729 [details] [diff] [review]
Add VKontakte protocol v.0.1

*** Original change on bio 1702 attmnt 1970 by unghost AT gmail.com at 2012-10-17 19:58:51 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353729 - Attachment is obsolete: true
*** Original post on bio 1702 by Alexander L. Slovesnik <unghost AT gmail.com> at 2012-10-17 20:04:38 UTC ***

(In reply to comment #9)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Comment on attachment 8353729 [details] [diff] [review] (bio-attmnt 1970) [details]
> > > Add VKontakte protocol v.0.1
> > > >diff --git a/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png b/chat/protocols/vkontakte/icons/prpl-vkontakte-32.png
> > > Where are these images from? Can we use them?
> > From http://blog.ragnaar.com/2009/05/ikonki-dlya-vkontakte.html
> > Do I need to ask permission to use them?
> It looks like it is implied they're free to use, but I don't read Russian (?)
> and there's no explicit license on that page. I think it would be best to ask
> (unless Florian has an opinion...I don't really know how media is licensed).

I've sent you letter with permission to use these icons by e-mail.
 
> > > >diff --git a/instantbird/content/account.js b/instantbird/content/account.js
> > > >     let protoId = this.proto.id;
> > > >     if (protoId == "prpl-irc" || protoId == "prpl-jabber" ||
> > > >-        protoId == "prpl-gtalk") {
> > > >+        protoId == "prpl-gtalk" ||
> > > >+        protoId == "prpl-vkontakte") {
> > > I just want to confirm to Vkontakte supports chatrooms, is that true?
> > No. I don't think that Vkontakte supports chatrooms. Should I undo this change?
> Yes, these changes have to do with joining chats. (That's why gtalk is listed,
> but not Facebook!)

I assume you want me to drop these changes everywhere in account.js and joinchat.js because there is no Facebook in these files.
*** Original post on bio 1702 at 2012-10-17 23:12:07 UTC ***

(In reply to comment #13)
> I've sent you letter with permission to use these icons by e-mail.
We got it, thanks.

> > > > I just want to confirm to Vkontakte supports chatrooms, is that true?
> > > No. I don't think that Vkontakte supports chatrooms. Should I undo this change?
> > Yes, these changes have to do with joining chats. (That's why gtalk is listed,
> > but not Facebook!)
> 
> I assume you want me to drop these changes everywhere in account.js and
> joinchat.js because there is no Facebook in these files.

Yes. Which it looks like you did in the newest patch... I'll try it shortly.
*** Original post on bio 1702 at 2012-10-17 23:38:14 UTC ***

For the record, we need to use vkmessenger.com and not vk.com since we don't support DNS SRV:

(from http://vk.com/help.php?page=jabber)
> Clients who do not support DNS records should indicate this as the server:
> vkmessenger.com.
Comment on attachment 8353738 [details] [diff] [review]
Add VKontakte protocol v.0.2

*** Original change on bio 1702 attmnt 1979 at 2012-10-17 23:39:54 UTC ***

Looks good, although I'd still like flo to review this as well.

I tested this and was able to connect, I don't have any contacts though...so I couldn't talk. :)
Attachment #8353738 - Flags: review?(clokep) → review+
Attachment #8353738 - Flags: review?(florian)
Comment on attachment 8353738 [details] [diff] [review]
Add VKontakte protocol v.0.2

*** Original change on bio 1702 attmnt 1979 at 2012-10-18 00:00:43 UTC ***

This looks great, thanks!
My only (trivial) review comment is that I think you no longer need this line: "Cu.import("resource:///modules/xmpp-xml.jsm");"
Attachment #8353738 - Flags: review?(florian) → review+
Whiteboard: [mentor=clokep] → [mentor=clokep][checkin-needed]
*** Original post on bio 1702 as attmnt 1993 by unghost AT gmail.com at 2012-10-19 18:12:00 UTC ***

With "Cu.import("resource:///modules/xmpp-xml.jsm");" dropped.
Comment on attachment 8353738 [details] [diff] [review]
Add VKontakte protocol v.0.2

*** Original change on bio 1702 attmnt 1979 by unghost AT gmail.com at 2012-10-19 18:12:15 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353738 - Attachment is obsolete: true
*** Original post on bio 1702 at 2012-10-27 02:31:04 UTC ***

Pushed as http://hg.instantbird.org/instantbird/rev/1a5b883bbd5c

Thanks!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [mentor=clokep][checkin-needed] → [mentor=clokep]
Target Milestone: --- → 1.3
There was missing email mapping information for this bug during the BIO to BMO merge, manually assigning this bug.
Assignee: bugzilla → unghost
Blocks: 1789724
You need to log in before you can comment on or make changes to this bug.