> > With this XEP approaching its way to draft status, I gave it a full > review. All in all it was quite readable and from a protocol/logic point of > view it doesn't look that hard to implement. >
Thanks for the compliment! First off some editorial points: > * It'd be nice if the glossary used definition lists (<dl/>). This would > improved rendering in XHTML and PDF and would make it consistent with other > XEPs. > * The introductory sentence of section 5 is missing an ending period. > Minor editorial corrections -- Thanks, will add those in. CHANGE #1: Minor edits Next my remaining points: > * The last point of section 4.6.2 ( > http://xmpp.org/extensions/xep-0301.html#attribute_values ) mentions > behavior in case a client receives forbidden values for attributes. Why not > just ignore the RTT message, if it violates the standard? Would be easier > to describe and to implement I think. If the other side send illegal values > there, clipping them to 0 there would hardly make it interoperate as > expected, or would it? > A very reasonable question to bring up, abut this specific sentence. None of the pre-existing implementations ever sends negative values, yet all of them already clips them to zero. So It has never shown up in testing except in forced situations. Now, we did run into some situations where a client had a bug in Unicode counting, and resulted in a count that caused text to be inserted slightly in the wrong position (off-by-1 error). So the legitimate question comes up: What's the lesser of evil (act upon these <rtt/> or reject these <rtt/>). On one hand, it would make a lot of sense to let the implementer decide whether to act upon these elements (and risk odd appearance of text), or to reject them (invoke 4.7.2 "loss of sync" whenever deciding to ignore an <rtt>) . Regardless of what's chosen, the recommended intermittent message refresh ( http://xmpp.org/extensions/xep-0301.html#message_refresh ) will automatically replace the whole message with the correct message, if any bug-induced text scrambling occurs. This is not why 4.7.2 was created, but it'll neatly cover this use case anyway. Clipping to 0 is the simpler of approach in pre-existing implementations. It is just a single extra line of code to clip to 0. While rejecting the <rtt> involve more code -- I checked the code -- In order to reject the <rtt>, I have to breaks the middle of processing, return from a function, tell the caller it failed, and to reject the rest of the <rtt> element. I'd need to add extra code to do that. Pre-existing rejection cases are done at the higher <rtt> layer level, without needing to look inside the nested elements (action elements) inside <rtt> to decide whether the <rtt> should be rejected or not. I'd rather not add situations where I must look inside action elements to decide on rejecting <rtt>. So, thus, you see, your suggested method would actually add a complication. Since it's so rare, I'd leave this as-is (simplest approach), to keep things simple. CHANGE #2: None > * Regarding section 6.1, Activating RTT: While I'm okay for only > *RECOMMENDING* (not REQUIRING) explicit discovery via disco/caps for 1-to-1 > chats, I'm not at all when it comes to RTT within MUCs. Matt pointed me to > XEP-0045, section 17.1.1 ( > http://xmpp.org/extensions/xep-0045.html#impl-service-traffic ), which > describes a way to discover what extended namespaces are allowed within a > room. I think the XEP should REQUIRE discovering the supported extended > namespaces and if RTT is among them, before RTT is send to a room. What an > RTT client is allowed to do if the MUC component doesn't support discovery > of allowed extended namespaces (i.e. returning <feature-not-implemented/>) > is up for debate. > In addition, if sending RTT without explicit discovery is allowed, the > protocol should provide a way to signal the sender of these RTT messages to > refrain from further sending RTT messages to the other side. I can imagine > in some scenarios, i.e. really low bandwidth or high latency scenarios, you > simply don't want this excessive traffic. > 17.1.1 sounds like a fair reference, a simple one-sentence (or two) modification to mention that if MUC is used, to comply with XEP-0045 section 17.1.1 and make sure urn:xmpp:rtt:0 is an allowable namespace. CHANGE #3: We could add a small amount of text to mention 17.1.1 of XEP-0045 > To sum up, it's a nicely written XEP which covers a lot different angles > including internationalization, privacy, relation to other XEPs. > When I get around, I'll try to implement it, at least experimentally, in > Swift. Thanks for the compliment! Feel free to contact me personally if you have specific questions. P.S. For open source XEP-0301 examples (conveniently written in Apache 2.0 license). These versions implement the recommended key press interval feature. Some of these are based on older versions of XEP-0301, but all of these interoperate with each other. Java: https://code.google.com/p/realjabber/source/browse/trunk/Java/src/RealTimeText.java C#: https://code.google.com/p/realjabber/source/browse/trunk/CSharp/RealTimeText.cs JavaScript: https://github.com/cvogler/trophyjs/blob/master/trophy.js Thanks, Mark Rejhon
