Can you add matrix.org as a supported platform as would be great

Can you expand a bit on how you'd like to see this integrated? It is not entirely clear to me whether it would be an additional protocol, or integrate in some other way. Thanks!

It would be great as it as an additional protocol aka IM Intergrating this

Moving to chat core.

I'm the project lead on Matrix - I think what satdav is asking for here is support for Matrix as another messaging protocol alongside IRC etc. For context: Matrix is an HTTP-based open standard for federated and decentralised messaging/voip. One use is as an alternative to IRC and XMPP; the API used for writing clients makes sending messages a simple HTTP PUT/POST and receiving messages in a room a simple long-lived GET, which works pretty well in an HTTP/2 world. The advantages over IRC and XMPP are that Matrix's main building block is decentralised signed conversation history and generally procides a much richer baseline set of capabilities (group chat, end-to-end encryption, voip, read receipts, push notifs, reliable message delivery etc). A good analogy could be that IRC is to Matrix as subversion is to git. Matrix is also focused on defragmenting existing communication protocols (hence the name Matrix). We provide an "application service" API that can be used for bridging to existing comms systems (IRC, XMPP, SIP, Lync etc) - for instance all of irc.mozilla.org and freenode are bridged into Matrix already, and the Mozillan community IT team are playing with it. See https://lwn.net/Articles/632572/ for more details on Matrix. Matrix is still in beta currently (since December) but is increasingle stable and mature and has a fairly wide range of client SDKs (js, python, iOS, Android etc) available. We haven't done a libpurple plugin one yet, but hopefully someone will; it should be pretty trivial. Our flagship SDK is probably matrix-js-sdk currently, which provides both a simple HTTP API wrapper as well as a nice state machine and object model for the client's lifecycle. If this sounds interesting and there's anything we can do to help on this, just say. thanks for considering it!

(In reply to Matthew Hodgson from comment #4) > I'm the project lead on Matrix - I think what satdav is asking for here is > support for Matrix as another messaging protocol alongside IRC etc. We'd certainly be interested in supporting this! (Whether the protocol plugin is provided as an addon, or shipped as one of the default set of protocols, would have to be decided by the team.) > Matrix is still in beta currently (since December) but is increasingle > stable and mature and has a fairly wide range of client SDKs (js, python, > iOS, Android etc) available. We haven't done a libpurple plugin one yet, but > hopefully someone will; it should be pretty trivial. Our flagship SDK is > probably matrix-js-sdk currently, which provides both a simple HTTP API > wrapper as well as a nice state machine and object model for the client's > lifecycle. For Thunderbird/Instantbird, I would recommend writing a JS protocol plugin. If you already have a JS implementation, this might be quite convenient. The relevant interfaces/APIs are in prplIProtocol.idl and prplIConversation.idl. jsProtoHelper.jsm provides a basic framework to build upon. To give you an idea, the existing JS protocols are here: https://dxr.mozilla.org/comm-central/source/chat/protocols If you have any questions, you're welcome to join #instantbird or #maildev on irc.mozilla.org!

This is an experimental adding a template for a new matrix protocol, with the basic "public rooms" test of matrix-js-sdk.

Comment on attachment 8809779 [details] [diff] [review] Experimental Patch (browser-matrix.min.js) Review of attachment 8809779 [details] [diff] [review]: ----------------------------------------------------------------- Looks like a pretty sane start! We'd need to hook up the rest of our methods to the SDK. Did you find any documentation about the SDK? Do you have to do polling or does it use a long-running connection? ::: chat/protocols/matrix/matrix-sdk.jsm @@ +21,5 @@ > + setTimeout: function(func, delay) { hwindow.setTimeout(func, delay); }, > +}; > +try { > + Services.scriptloader > + .loadSubScript("chrome://prpl-matrix/content/sdk.js", context, "UTF-8"); Doesn't look like you included this file? (What is this file?) You also have a browser-matrix.min.js file which seems unused. You might be able to use require now (I had some luck doing that in attachment 8725983 [details] [diff] [review] for bug 1141674, see the use of "Loader" in chat/protocols/facebook/facebook-mqtt.jsm). ::: chat/protocols/matrix/matrix.js @@ +10,5 @@ > +XPCOMUtils.defineLazyGetter(this, "_", () => > + l10nHelper("chrome://chat/locale/matrix.properties") > +); > + > +XPCOMUtils.defineLazyModuleGetter(this, "MatrixSDK", Why getting this lazy?

Comment on attachment 8809779 [details] [diff] [review] Experimental Patch (browser-matrix.min.js) Review of attachment 8809779 [details] [diff] [review]: ----------------------------------------------------------------- > Looks like a pretty sane start! We'd need to hook up the rest of our methods to the SDK. Did you find any documentation about the SDK? Do you have to do polling or does it use a long-running connection? The doc for the js API is https://matrix-org.github.io/matrix-js-sdk/index.html There is also more general doc on https://matrix.org/, such as https://matrix.org/docs/spec/intro.html But I did not find time to read this in details :-( ::: chat/protocols/matrix/matrix-sdk.jsm @@ +21,5 @@ > + setTimeout: function(func, delay) { hwindow.setTimeout(func, delay); }, > +}; > +try { > + Services.scriptloader > + .loadSubScript("chrome://prpl-matrix/content/sdk.js", context, "UTF-8"); chrome://prpl-matrix/content/sdk.js is browser-matrix.min.js (see how the packaging is defined in chat/protocols/matrix/jar.mn), which is the minified file for use in browser, downloaded from https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.6.4 Indeed it would be best to do require("matrix-js-sdk"), I was not sure I could use it here but I should give it a try. ::: chat/protocols/matrix/matrix.js @@ +10,5 @@ > +XPCOMUtils.defineLazyGetter(this, "_", () => > + l10nHelper("chrome://chat/locale/matrix.properties") > +); > + > +XPCOMUtils.defineLazyModuleGetter(this, "MatrixSDK", browser-matrix.min.js seems big and matrix.js seems loaded each time one wants to add a new account, even a non-matrix one (at least on Thunderbird). So I preferred to make it lazy, but not sure whether that's important.

Another version, using require and commonjs modules.

This is really cool! To answer :clokep's questions from earlier - the JS SDK's API doc is at http://matrix-org.github.io/matrix-js-sdk/0.6.4/index.html, and the actual Matrix spec itself is at https://matrix.org/docs/spec and https://matrix.org/docs/api. The baseline API for Matrix is still keep-it-simple/stupid long-polling via HTTP - however, in future we're expecting more advanced optional transports to emerge like the https://github.com/matrix-org/matrix-doc/blob/master/drafts/websockets.rst websockets one. :fredw - we'd be very happy to help you on this in #matrix-dev:matrix.org if you're not already talking to the core team! :)

Here is an updated patch that implements the following basic features: - Matrix account creation. - Connection/Deconnection to/from a Matrix server using a login/password. - Joining a room and getting the list of participants. - Receiving/sending messages. It includes the following changes to matrix-js-sdk: https://github.com/matrix-org/matrix-js-sdk/pull/282/files https://github.com/matrix-org/matrix-js-sdk/pull/286/files https://github.com/matrix-org/matrix-js-sdk/pull/284/files You can test it locally by installing a synapse server: https://github.com/matrix-org/synapse By default, synapse generates self-signed certificate and hence Mozilla chat client will refuse connection by default. There is probably a pref to change that behavior but otherwise you can set "no_tls: True" in the homeserver.yaml file. You can then create a new user: register_new_matrix_user -c homeserver.yaml http://localhost:8008 New user localpart: login Password: passwd Confirm password: passwd Success! The data to use for the Chat Account Wizard are then: Username: login Password: passwd Server: http://localhost Port: 8008

(In reply to Frédéric Wang (:fredw) from comment #12) > By default, synapse generates self-signed certificate and hence Mozilla chat > client will refuse connection by default. There is probably a pref to change > that behavior but otherwise you can set "no_tls: True" in the > homeserver.yaml file. You can then create a new user: We'll probably need to support the bad-cert handler we have (which gives you a link in the account manager to add an exception), check out handleBadCertificate on the prplIAccount object. irc.js implements this at https://dxr.mozilla.org/comm-central/rev/dd4587a78cfd200881956427c8d3fd468aaf9758/chat/protocols/irc/irc.js#731-736 Feel free to ping me on GitHub @clokep if you need my opinion / thoughts / just want to notify on me on a PR.

Comment on attachment 8810496 [details] [diff] [review] Part 3 - Implement new Matrix protocol (WIP) Review of attachment 8810496 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/protocols/matrix/matrix.js @@ +32,5 @@ > +MatrixConversation.prototype = { > + __proto__: GenericConvChatPrototype, > + sendMsg: function(aMsg) { > + this._account._client.sendTextMessage(this._roomId, aMsg); > + this.updateParticipants(); That updateParticipants call is a mistake, it should not be present. @@ +35,5 @@ > + this._account._client.sendTextMessage(this._roomId, aMsg); > + this.updateParticipants(); > + }, > + get room() { return this._account._client.getRoom(this._roomId); }, > + updateParticipants: function() { Probably, we should have one function generating the initial list of participants and then let add or remove participant one by one after "RoomMember.membership" events...

One of the problem I had when writing the patch was to make loginWithPassword work. It seems I'm not the only one as there are three similar issues opened on GitHub (for now I'm doing the workaround https://github.com/matrix-org/matrix-js-sdk/issues/130#issuecomment-213819684): https://github.com/matrix-org/matrix-js-sdk/issues/3 https://github.com/matrix-org/matrix-js-sdk/issues/130 https://github.com/matrix-org/matrix-js-sdk/issues/132 Also, the doc about high-level events is not really helpful. One has to end up reading the code to check which event to listen and how they are used (or otherwise use low-level events and read the Matrix client-client API): https://github.com/matrix-org/matrix-js-sdk/issues/246 Do you have plan to address these issues soon? It essentially seems to be improving the doc and I believe it would really help SDK users. (As I mentioned above, I also had https://github.com/matrix-org/matrix-js-sdk/issues/250 but it's probably less important)

Hi :fredw, I'm one of the core developers on Matrix. The workaround you mentioned ( https://github.com/matrix-org/matrix-js-sdk/issues/130#issuecomment-213819684 ) is the main way people authenticate using the JS SDK. There are a number of reasons why this is the case, primarily because there are many ways a user may *register* for an account, and the JS SDK is not aware of them all. Due to that, it's not possible for the JS SDK to know when an API call to /register (and by extension /login) will return an access_token for the user. We can improve this, but since the workaround is so easy, we've not devoted further time working out the details. We can clarify this in the README and/or the JSDoc for the login() function. The docs for EventEmitter events assumes a degree of prior knowledge about Matrix events: http://matrix.org/docs/spec/client_server/r0.2.0.html#event-structure . In almost all cases, it is preferable to listen for the high-level form rather than the raw "event" form or else you will end up performing duplicate work which the JS SDK already does for you. For example, https://matrix-org.github.io/matrix-js-sdk/0.6.4/module-client.html#~event:MatrixClient%2522RoomMember.powerLevel%2522 lets you know when the privileges of a room member changes, but to extract that from the raw event http://matrix.org/docs/spec/client_server/r0.2.0.html#m-room-power-levels is a lot more involved as you need to respect defaults, etc. We're always open to documentation improvements. Please let us know on https://matrix.to/#/!XqBunHwQIXUiqCaoxq:matrix.org if you hit any areas you think we can improve on.

thanks kegan. fredw: i really can't overstate that the best way to get support on this is to come talk to us on #matrix-dev:matrix.org at the time rather than suffer alone :D The doc is still fairly young, and we'll jump on complaints where it's not as clear as it should be!

Comment on attachment 8810495 [details] [diff] [review] Part 2 - Add basic setup for a new Matrix protocol. Review of attachment 8810495 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/protocols/matrix/matrix-sdk.jsm @@ +13,5 @@ > +this.EXPORTED_SYMBOLS = ["MatrixSDK"]; > + > +// Set-up loading so require works properly in CommonJS modules. > +// TODO: Check readyState? https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.importGlobalProperties#hiddenDOMWindow > +var hwindow = Services.appShell.hiddenDOMWindow; Drive-by comment: I'm a bit concerned at your use of the hidden window, with its chrome privileges. Generally this would be r- for a backend protocol which by design is separated from the UI. What is it actually needed for? The setTimeouts and setIntervals below certainly work without it.

Comment on attachment 8810495 [details] [diff] [review] Part 2 - Add basic setup for a new Matrix protocol. Review of attachment 8810495 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/protocols/matrix/matrix-sdk.jsm @@ +13,5 @@ > +this.EXPORTED_SYMBOLS = ["MatrixSDK"]; > + > +// Set-up loading so require works properly in CommonJS modules. > +// TODO: Check readyState? https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.importGlobalProperties#hiddenDOMWindow > +var hwindow = Services.appShell.hiddenDOMWindow; Yes, currently this is a quick hack and I wish we can remove it in the final patch. In general, matrix-js-sdk and its dependencies seem to assume the presence of functions available in a browser context. For now I used console and XMLHttpRequest and for timers we can probably use https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Timer.jsm (I haven't tried yet). There are other places where "window" or "document" functions are used (e.g. localStorage). I haven't checked the details.

(In reply to Frédéric Wang (:fredw) from comment #22) > Comment on attachment 8810495 [details] [diff] [review] > Part 2 - Add basic setup for a new Matrix protocol. > > Review of attachment 8810495 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: chat/protocols/matrix/matrix-sdk.jsm > @@ +13,5 @@ > > +this.EXPORTED_SYMBOLS = ["MatrixSDK"]; > > + > > +// Set-up loading so require works properly in CommonJS modules. > > +// TODO: Check readyState? https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.importGlobalProperties#hiddenDOMWindow > > +var hwindow = Services.appShell.hiddenDOMWindow; > > Yes, currently this is a quick hack and I wish we can remove it in the final > patch. In general, matrix-js-sdk and its dependencies seem to assume the > presence of functions available in a browser context. For now I used console > and XMLHttpRequest and for timers we can probably use > https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/ > Timer.jsm (I haven't tried yet). There are other places where "window" or > "document" functions are used (e.g. localStorage). I haven't checked the > details. Somes of these are importable into the backend using Components.utils.importGlobalProperties: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.importGlobalProperties

My previous patch already used: - Cu.importGlobalProperties for XMLHttpRequest - Console.jsm for console Here is an updated patch that removes the hidden window. It uses Timer.jsm for: - setTimeout - setInterval - clearTimeout - clearInterval The "global.document" is used in matrix-js-sdk in various places, but it is tested before being used. It does not seem to be needed in my testing. The same seems to be true for "global.localStorage" so I've removed my definition from now. It seems that Matrix allows to specify our own implementation. Patrick mentioned https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/NsIDOMStorageManager Finally "document" is used by browser-request for a IE hack and otherwise window.location is needed: https://github.com/iriscouch/browser-request/blob/master/index.js#L403 For now my workaround is to define a dummy global variable "location" to make is_crossDomain return without error although it now sends "CORS request rejected" error messages. Maybe we can find a replacement for this request API matrix wants, see https://github.com/matrix-org/matrix-js-sdk/blob/master/lib/matrix.js#L72 Otherwise we can just modify the code to force is_crossDomain to always return true.

I just tested this briefly and it seems to work enough to get into a room! We likely want to move the "server" variable into a username split (it's also not 100% clear to me what to provide as a username vs. the server, the username seems to already have a server in it...) Looking over the code, it looks fairly reasonable (creating the Matrix client and then hooking up events and handling). There's definitely some things I'd change in there, but I think it's the right approach!

(In reply to Patrick Cloke [:clokep] from comment #25) > I just tested this briefly and it seems to work enough to get into a room! Did you try sending/receiving messages? > We likely want to move the "server" variable into a username split (it's > also not 100% clear to me what to provide as a username vs. the server, the > username seems to already have a server in it...) In the Matrix server of my company, the server used to log in is different from the domain in user & room ids. The latter is retrieved from MatrixClient::getDomain() after logging in. I'm not exactly sure how username is handled in Matrix either. > Looking over the code, it looks fairly reasonable (creating the Matrix client and then hooking up events and handling). There's definitely some things I'd change in there, but I think it's the right approach! See also comment 17

Addressing comments 17...

I'm importing the latest release v0.7.0 of Matrix JS SDK (https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.7.0), that includes my changes. It does not seem to break anything here.

IIUC, Mozilla has two concepts: 1) "chat" for a general channel with several participants. You need to mention a nickname when writing a message in order to send a notification to the corresponding participant. 2) "conversation" for discussion between two users. Each message sends a notification to your interlocutor, you don't need to mention the nickname. Matrix seems to have similar concepts. However "conversation" is a bit different: a) "conversations" are implemented as a normal room. b) it is possible to open several "conversations" with a same interlocutor. c) it seems possible to invite other people to a "conversation" between 2 users. Once a room for a "conversation" is created, I can use the room id to communicate normally in Thunderbird using the "chat" concept, but that's not really the standard approach. @Kegan Any thought on this? Do you know which API is provided by the JS SDK to manage such "conversation"?

(In reply to Frédéric Wang (:fredw) from comment #29) > IIUC, Mozilla has two concepts: > > 1) "chat" for a general channel with several participants. You need to > mention a nickname when writing a message in order to send a notification to > the corresponding participant. > 2) "conversation" for discussion between two users. Each message sends a > notification to your interlocutor, you don't need to mention the nickname. > > Matrix seems to have similar concepts. However "conversation" is a bit > different: > a) "conversations" are implemented as a normal room. > b) it is possible to open several "conversations" with a same interlocutor. > c) it seems possible to invite other people to a "conversation" between 2 > users. There's probably no exact correspondence. How you map these concepts across depends on what the most common use case is. Note if you plan to add contacts to the contact list, starting a conversation with them will take you to type 2). In case c) you could open a MUC ("chat") after the invite is accepted with the appropriate participants in it. Handling the slightly surprising b) is likely the trickiest part if you want to support it. Not sure what this is used for - the closest existing comparison might be XMPP resources?

@fredw As aleth has said, there is no exact correspondence, it depends what use cases you are aiming for. I agree that b) is slightly surprising: it's a result of having every room be a MUC ("chat") effectively. We have no restrictions on who can invite who, nor that there is a single primary room for correspondence between users. Other clients, such as Riot, add a flag to the room to indicate it is a "Direct Message" which is then treated as a "conversation" between 2 users (same notification semantics as you describe). When the UI populates the "People" section on Riot-Web, it bases that on whether this flag is present. When you "Start a Chat" with a single participant, if there is an existing flagged DM room with that user then the UI takes you to that conversation, else it creates a new DM room. For more information, see http://matrix.org/speculator/spec/HEAD/client_server/unstable.html#direct-messaging for the precise details.

Thanks for the information Kegan, I think this "Direct Message" is what we want. We'll have to take a look into this. Is it already available in the JS SDK?

Yes, there are 3 things you need to do: - When you create a room that is intended to be a Direct Message, set the flag 'is_direct' to true. In the JS SDK, this is `MatrixClient.createRoom({ is_direct: true });`: https://github.com/matrix-org/matrix-react-sdk/blob/be232bc1dae463e2ca90fb0710a0220ac068abe9/src/createRoom.js#L63 - When you receive an invite to a room, check if the room is intended to be a Direct Message by inspecting the `m.room.member` event for the `is_direct` flag: https://github.com/matrix-org/matrix-react-sdk/blob/b5cd540e271498e38661d127212b62f544c95603/src/utils/DMRoomMap.js#L104 - When you create a Direct Message room, or receive an invite to a Direct Message room, or if the user explicitly wants to set a room to be a Direct Message room, get/set the account data according to the specification: https://github.com/matrix-org/matrix-react-sdk/blob/be232bc1dae463e2ca90fb0710a0220ac068abe9/src/Rooms.js#L95 You can then display the list of Direct Message rooms by inspecting your account data event.

Sorry for the weird format...I applied your patches and started making inline notes as comments. I figured the easiest way to get the feedback here was to upload the patch. Note that this is based on the patches uploaded on November 14, 2016.

fredw and I also had a conversation on IRC, tl;dr: I'd be happy getting this into a little bit better shape and landing it preffed off. We can then iterate on it and likely propose it as a GSoC project to finish this up (hopefully with some support for mentoring from the Matrix.org team too!) 10:22:15 AM - fredw: clokep_work, aleth-mob: Hi. So we finally got a reply about this "direct message" concept of matrix... 10:22:41 AM - clokep_work: I saw it, but haven't read it yet. 10:22:48 AM - fredw: I was wondering, do you plan to propose this as a GSOC project this summer? 10:29:09 AM - clokep_work: fredw: Maybe, is there even enough there for a GSoC project? 10:34:38 AM - fredw: clokep_work: I would say yes. At least it seems there is a bunch of things to that remains to do if we want to map matrix features to mozilla client (+ some extra features like knowing the last messages read by people, sharing files, etc) 10:35:20 AM - fredw: and maybe it will be necessary to work on the matrix JS SDK too... 10:36:42 AM - clokep_work: fredw: OK. I'd be quite happy to have it, as long as it seems like a project a student can work on for... 8 weeks. 12:05:56 PM - clokep_work: fredw: Just read that email, sounds pretty reasonable. :) 12:08:15 PM - fredw: clokep_work: Yes, I think we now have the correspondence between Matrix features and Mozilla chat features. We "just" have to implement it :-) 12:09:08 PM - fredw: Well, actually I see matrix/riot has more features but I'd already happy to have the basic chat features working in Mozilla. 12:10:16 PM - fredw: Another thing I'm not clear is how we will manage the synchronization between the chat history (saved on the server VS cached by Mozilla). 12:12:21 PM - clokep_work: fredw: I'd definitely be OK landing a version that's "basic" and then adding more features. :) 12:12:34 PM - clokep_work: I'm not sure how to do that either, we might need to add things to the logger framework if we want to fully sync. 12:14:47 PM - fredw: clokep_work: maybe I can try rebasing my patch and have it reviewed and landed (with the matrix pref option disabled)? 12:15:34 PM - fredw: So that we have a base implementation for future work 12:16:50 PM - clokep_work: fredw: Maybe! I have some changes I made to it (mostly notes / comments). 12:16:53 PM - clokep_work: I should probably upload it. 12:20:10 PM - fredw: clokep_work: sounds good yes. I guess I can try to take a look this week-end...

Comment on attachment 8830069 [details] [diff] [review] Some inline review comments Review of attachment 8830069 [details] [diff] [review]: ----------------------------------------------------------------- Thank you Patrick. I'm replying to some inline comments. When I have time I'll merge your patch, test again and ask for review. ::: chat/protocols/matrix/matrix.js @@ +146,4 @@ > } > }); > this._client.on("RoomMember.membership", function(event, member, oldMembership) { > + // TODO This could likely be made more efficient. I believe the main diff from the patch of 2016-12-03 is to improve how the list of participants is updated. @@ +208,5 @@ > let roomIdOrAlias = aComponents.getValue("roomIdOrAlias").trim(); > let domain = this._client.getDomain(); > if (!roomIdOrAlias.endsWith(":" + domain)) > roomIdOrAlias += ":" + domain; > + // XXX Where does this hard-coded list come from? See https://matrix.org/docs/spec/intro.html#room-structure https://matrix.org/docs/spec/intro.html#room-aliases @@ +250,5 @@ > get iconBaseURI() { return "chrome://prpl-matrix/skin/"; }, > getAccount: function(aImAccount) { return new MatrixAccount(this, aImAccount); }, > > options: { > + // XXX Should this default to matrix.org? I think the current implementation assumes that we have a login & password. We should probably also handle the case of matrix servers that accept guests.

I'm merging Patrick's patch into mine. As discussed, let's on comment 35 let's try and land this very basic support (preferred off) and iterate in follow-up bugs.

Comment on attachment 8816699 [details] [diff] [review] Part 1 - Import Matrix JS SDK from github.com/matrix-org Review of attachment 8816699 [details] [diff] [review]: ----------------------------------------------------------------- I'm not really sure what to review here. I suspect we should include a file that says what version (commit) of this SDK was included and how to update it in the future. (And what, if any, modifications were made.)

Comment on attachment 8811395 [details] [diff] [review] Part 2 - Add basic setup for a new Matrix protocol Review of attachment 8811395 [details] [diff] [review]: ----------------------------------------------------------------- ::: chat/protocols/matrix/jar.mn @@ +1,3 @@ > +chat.jar: > +% skin prpl-matrix classic/1.0 %skin/classic/prpl/matrix/ > + skin/classic/prpl/matrix/icon32.png (icons/prpl-matrix-32.png) Where are these images from? What's their license?

Comment on attachment 8831335 [details] [diff] [review] Part 3 - Implement new Matrix protocol Review of attachment 8831335 [details] [diff] [review]: ----------------------------------------------------------------- It looks to me like we should just remove that debug code and this is good enough for initial testing. ::: chat/protocols/matrix/matrix.js @@ +125,5 @@ > + startClient: function() { > + let account = this; > + // this._client.on("event", function(event) { > + // this.DEBUG("Low-level event:", event); > + // }); We should remove this before checking in.

> I'm not really sure what to review here. I suspect we should include a file that says what version (commit) of this SDK was included and how to update it in the future. (And what, if any, modifications were made.) I've added a README file with explanations. For completeness, I also added missing copyright headers to some files. I think this part is ready now.

> Where are these images from? What's their license? IIRC, I had generated them myself using the icons from the matrix website. Anyway, just to be safe I re-regenerated icons using https://github.com/matrix-org/matrix.to/blob/master/img/matrix-logo.svg as a basis. This repo is licensed under Apache 2. I also added a README.

> We should remove this before checking in. Done

Rebasing to fix conflict in chat/locales/jar.mn

Rebasing to fix conflict in chat/locales/jar.mn

https://hg.mozilla.org/comm-central/rev/bd5460ae6c1ca6be12e12fbe804ef8d5a12255d3 https://hg.mozilla.org/comm-central/rev/b99d93143c49b92aaf252143ff1a64a8c38ec28b https://hg.mozilla.org/comm-central/rev/f6a8a506a1c95767ae64e65101a83ea866dd5cdf

Thanks for taking care of updating these patches! I'm excited to play with this. :)

Moving Matrix bugs to the new Matrix component.