(In reply to comment #0)
> * migrate the awkward chatroom properties onto the NamedChatroom interface, or
> the Text interface, or some separate interface

These should not be on NamedChatroom, unless we want to count persistent chat rooms with non-human-readable names as named...

Any future chatroom API should also address the requirements of Skype chatrooms, which have a globally unique identifier, and persist for as long as they have members, but do not have any *human-readable* identifier.

The only way to join a Skype chatroom is to be invited by a member of it, so the identifier doesn't need to be a requestable property.

http://git.collabora.co.uk/?p=rtcom-telepathy-glib.git;a=blob;f=rtcom-telepathy-glib/Channel_Interface_Persistent.xml is the solution used in Maemo 5.

The functionality of http://git.collabora.co.uk/?p=rtcom-telepathy-glib.git;a=blob;f=rtcom-telepathy-glib/Channel_Interface_Skype_GroupChat.xml should also be considered.

Note that Google's PMUCs have opaque identifiers like Skype's.

(In reply to comment #2)
> The only way to join a Skype chatroom is to be invited by a member of it, so
> the identifier doesn't need to be a requestable property.

I believe that this isn't true — you can leave and re-join a room.

Here's a first cut at an interface for representing room names better: http://people.freedesktop.org/~wjt/telepathy-spec-room/spec/Channel_Interface_Room.html

It doesn't deal with anything from Skype.GroupChat (Leave() and properties for the Topic).

One problem that's come to mind is that you can't actually tell whether an XMPP MUC JID is human-readable or not.

If you're the one creating the room, this isn't a problem, because Gabble knows.

If you're invited to the room, then I think the MUC could reasonably include a <unique/> tag in the invitation. I shall post to muc@ accordingly.

If it's Google Talk, we special-case it anyway.

If you explicitly join a room that has a unique name... then you're stuck. But if the channel requester includes Name: "" in the request alongside TargetID: "lrcgsnthzvwm@conference.jabber.org" then Gabble can take this as a hint that the client thinks this MUC's JID is opaque.

Okay, so I took this from Will and updated it. I fixed some of the FIXMEs over the spec, added a Subject property and just more misc documentation.

Here's some pretty HTML:

http://people.freedesktop.org/~jonny/telepathy-spec-room/spec/Channel_Interface_Room.html

Still to do:

 * There's a FIXME there that I need to work out.

 * and a TODO about XMPP ad-hoc MUCs. I haven't looked up what they are yet.

Other than that, thoughts? I've done a quick look through Empathy and it appears only the last three TP properties of C.T.Text (subject, subject-contact, subject-timestamp) are only ever used.

(In reply to comment #7)
>  * and a TODO about XMPP ad-hoc MUCs. I haven't looked up what they are yet.

They're called instant rooms in XEP-0045; they're just like GTalk ones except you ask the server for the UUID rather than generating it yourself.

http://xmpp.org/extensions/xep-0045.html#createroom-instant

(In reply to comment #7)
>  * There's a FIXME there that I need to work out.

Now removed.

> FIXME: should that second RCC have THT=None ?

No, I think it should be left out; the THT is really None on MSN, and Room everywhere else. I think explicitly asking for THT=None and getting THT=Room is unnecessarily confusing, but not specifying a THT makes more sense.

(Butterfly could have an undocumented, old-Empathy-compatible, bonus RCC if necessary?)

> { ...ChannelType: ...Text,
>   ...TargetHandleType: Room,
>   ...TargetID: "lkajdskjakjsadhkjsh@conference.jabber.org",
>   ...RoomID: ""
> }

What happens if I specify a RoomID that conflicts with the TargetID?

Would a boolean "target ID is human-readable" flag make more sense, even if it's, strictly speaking, redundant?

> # Status — u (Contact_Handle)

"Actor" or "Set_By", surely?

Bonus penguin points for getting the D-Bus type "sux" into the spec, but would we prefer the actor to be a normalized string identifier?

(In reply to comment #10)
> > { ...ChannelType: ...Text,
> >   ...TargetHandleType: Room,
> >   ...TargetID: "lkajdskjakjsadhkjsh@conference.jabber.org",
> >   ...RoomID: ""
> > }
> 
> What happens if I specify a RoomID that conflicts with the TargetID?

I've now specified that the channel fails with InvalidArgument and made it explicit how TargetHandle and TargetID is to be used with RoomID.

> Would a boolean "target ID is human-readable" flag make more sense, even if
> it's, strictly speaking, redundant?

No I don't see the point. We need to have this RoomID property so let's just use it.

> > # Status — u (Contact_Handle)
> 
> "Actor" or "Set_By", surely?

Oops! Fixed.

> Bonus penguin points for getting the D-Bus type "sux" into the spec, but would
> we prefer the actor to be a normalized string identifier?

Meh, okay, done.

Branch and HTML spec in the same place.

I've also added a note about how to request a new XMPP uniquely named room.

So all TODOs are done. More comments please!

I think this is basically good to merge as a draft, with or without the changes below.

> RoomID — s, Read only
>
> The human-readable identifier of a chat room. Note that this property (and
> perhaps also Server) should be sufficient in a channel request to join the room.

"Note that if non-empty, this property..." might be clearer?

> Subject — (ssx)

This seems like a namespace grab, could we make it a type called Room_Subject? That'd become TP_STRUCT_TYPE_ROOM_SUBJECT in telepathy-glib or Tp::RoomSubject in telepathy-qt4. The property can stay as Subject though.

It might be worth saying explicitly that the Subject should be ("", "", 0) in protocols that don't have subjects (MSN?).

Cross-references:

- Subject should have a tp:rationale saying it replaces the subject, subject-contact and subject-timestamp Telepathy.Properties of Text channels

- Those Telepathy.Properties should cross-reference to Chan.I.Room.Subject

- Chan.T.RoomList.GotRooms should gain a "room-id" key in its a{sv}, whose value is defined to be the Chan.I.Room.RoomID, with a link

- Either Chan.T.RoomList.GotRooms should gain a "server" key for completeness, or the text should say that clients may assume that the Chan.I.Room.Server is the same as the Chan.T.RoomList.Server

- Chan.T.RoomList.GotRooms' "subject" a{sv} key should link to Chan.I.Room.Subject

(In reply to comment #13)
> "Note that if non-empty, this property..." might be clearer?

Done.

> This seems like a namespace grab, could we make it a type called Room_Subject?
> That'd become TP_STRUCT_TYPE_ROOM_SUBJECT in telepathy-glib or Tp::RoomSubject
> in telepathy-qt4. The property can stay as Subject though.

Done.

> It might be worth saying explicitly that the Subject should be ("", "", 0) in
> protocols that don't have subjects (MSN?).

Done.

> - Subject should have a tp:rationale saying it replaces the subject,
> subject-contact and subject-timestamp Telepathy.Properties of Text channels

Done.

> - Those Telepathy.Properties should cross-reference to Chan.I.Room.Subject

Done.

> - Chan.T.RoomList.GotRooms should gain a "room-id" key in its a{sv}, whose
> value is defined to be the Chan.I.Room.RoomID, with a link

Done.

> - Either Chan.T.RoomList.GotRooms should gain a "server" key for completeness,
> or the text should say that clients may assume that the Chan.I.Room.Server is
> the same as the Chan.T.RoomList.Server

I added the server key.

> - Chan.T.RoomList.GotRooms' "subject" a{sv} key should link to
> Chan.I.Room.Subject

Done.

There could be other types of channels with ad-hoc participation and opaque IDs, like calls. All Skype calls have unique IDs. Deemphasize the chat affinity of this interface, as it does not seem to be bound to anything specific to text channels?

If this is going to be a generic interface for persistently identified channels, it could be used in channel handover (bug #28378) for cases when channel identities can be linked. It's not practical to use object paths for linking calls or chats over different protocols served by different CMs. Protocol-specific IDs, on the other hand, are likely to be used by underlying protocol pairs, so there would be no magic inter-CM knowledge sharing necessary.

(In reply to comment #16)
> If this is going to be a generic interface for persistently identified
> channels, it could be used in channel handover (bug #28378) for cases when
> channel identities can be linked. It's not practical to use object paths for
> linking calls or chats over different protocols served by different CMs.
> Protocol-specific IDs, on the other hand, are likely to be used by underlying
> protocol pairs, so there would be no magic inter-CM knowledge sharing
> necessary.

So what we could do is: split this into Channel.Interface.Identifier, with the pair of maybe-human-readable properties (which on Skype calls would both be blank), and a Channel.Interface.Room with just the Subject (and later we could glue member rôles and permissions and stuff on there too?). How does that sound?

This was merged, so there is no branch.

(In reply to comment #15)
> There could be other types of channels with ad-hoc participation and opaque
> IDs, like calls. All Skype calls have unique IDs. Deemphasize the chat affinity
> of this interface, as it does not seem to be bound to anything specific to text
> channels?

It's called Room and not specific to Text channels, so I think that's OK. When we say "Room" I think it's OK to mean "identifiable thing you can participate in". On XMPP, Muji calls take place in a chatroom (which also has a text component).

It'd be slightly astonishing to use Handle_Type_Room for Skype calls, but they could work just like Skype group chats as described in the introductory docstring (Handle_Type_None, TargetID = the opaque ID, this interface, empty RoomID to indicate that the TargetID is not human-readable).

I'm unsure about Chan.I.Identifier - surely TargetID is the identifier, and the things that you propose to move to Chan.I.Identifier are extras to aid in interpretation/display, making the name rather strange?

Over on Bug #26866 there's discussion of whether Chan.I.Addressing should implicitly only be for contacts. I think it probably should, and we could add a RoomURI property on this interface if we want to be able to reference (XMPP and IRC) rooms by URI.

Additional spec issues:

* Needs a SubjectChanged signal! (or the new generic PropertiesChanged)

Implementation status:

* Gabble is the complicated case, Bug #31470
* Butterfly and Skype are trivial - they just need empty, unrequestable properties (Skype might also have a subject, but that's easy to add) - so I don't think they need trial implementations
* IRC is strictly simpler than Gabble so I don't think it needs a trial implementation either

(In reply to comment #18)
> It's called Room and not specific to Text channels, so I think that's OK. When
> we say "Room" I think it's OK to mean "identifiable thing you can participate
> in".

I agree.

(In reply to comment #18)
> It'd be slightly astonishing to use Handle_Type_Room for Skype calls, but they
> could work just like Skype group chats as described in the introductory
> docstring (Handle_Type_None, TargetID = the opaque ID, this interface, empty
> RoomID to indicate that the TargetID is not human-readable).

Well, Skype calls are implicitly "rooms" (except it might be useful to represent p2p calls as such, and migrate them to a Conference channel when they grow more participants).
But I also see a need in opaque identification of channels that are not typical "rooms", e.g. a one-to-one cellular call in a handover. Doesn't the combination of TargetHandleType = Handle_Type_None and TargetID = "opaque_id" violate earlier assumptions about these properties?

(In reply to comment #21)
> But I also see a need in opaque identification of channels that are not typical
> "rooms", e.g. a one-to-one cellular call in a handover. Doesn't the combination
> of TargetHandleType = Handle_Type_None and TargetID = "opaque_id" violate
> earlier assumptions about these properties?

Right, that's not allowed - Handle_Type_None means the channel isn't identifiable by its TargetID, so it's not consistent with having a non-empty TargetID.

Another option might be an opaque ConversationID like I proposed in Bug #26838. I suppose the semantic distinction here (in my mind, at least) is that TargetHandle, TargetHandleType and TargetID point to a "thing" (either a contact or a room) that is the other participant in this conversation, whereas ConversationID identifies the conversation itself.

To be honest I don't think I understand the requirements for the handover case you describe, and I suspect that we're solving different problems.

(In reply to comment #22)
> Another option might be an opaque ConversationID like I proposed in Bug #26838.
> I suppose the semantic distinction here (in my mind, at least) is that
> TargetHandle, TargetHandleType and TargetID point to a "thing" (either a
> contact or a room) that is the other participant in this conversation, whereas
> ConversationID identifies the conversation itself.
> 
> To be honest I don't think I understand the requirements for the handover case
> you describe, and I suspect that we're solving different problems.

No, ConversationID suits me perfectly, thank you.

(In reply to comment #23)
> No, ConversationID suits me perfectly, thank you.

On a better thought, that is too early to say. The requirement to make IDs unique over time may prove too strong to make it usable with protocol IDs. We can think of something later; e.g. inventing an ID property specific for the handover case.

At the same time as undrafting this, we should extend the Chan.T.Text top-level description to explain how opaquely-named rejoinable chatrooms (Skype) work.

So I implemented Room.DRAFT in Gabble as shown in bug #32611.

I've also done a bit more work on this. Check out my branch. The changes in this spec branch will break my outstanding Gabble branch, but it will be trivial enough to update it.

(In reply to comment #26)
> The changes in
> this spec branch will break my outstanding Gabble branch, but it will be
> trivial enough to update it.

FYI, I've updated my gabble branch to incorporate these changes (on top of my previous room branch, but not a blocker). See the dependent bug for more details.

14:07 < jonnylamb> smcv: oh, also, does all this Subject stuff really belong in 
                   Room?
14:07 < jonnylamb> You can have 1-1 chats with subjects in Skype.
14:08 < smcv> fun fact: Telepathy 0.old had Chan.I.Subject
14:08 < Robot101> telepathy 0.8 had subject as a separate interface! from the 
                  department of "it was all beter 5 years ago" :P
14:08 < Robot101> smcv: ^5
14:08 < smcv> jonnylamb: I think you're probably right, we should bring back 
              Chan.I.Subject
14:09 < jonnylamb> hmph

If a protocol has the concept of a room subject, but has a distinction between "no subject" and "subject = ''", or "subject not known yet" and "subject = ''", would Present be set at all times, or only when a subject is known, or only when a non-empty subject is known? My guess would be that Present would be set at all times (so it MUST NOT change), 

Does Can_Set mean "the protocol allows me to try to set the subject" or "I am sufficiently privileged to actually set the subject"? If the former, it MUST NOT change; if the latter (which I assume is what you meant), obviously it might change. However, you can't actually tell the latter reliably in XMPP. I researched some MUC implementations a while ago, and they don't all interpret the informational flag in the same way :-( You can get pretty close, as Gabble currently does for the Tp Properties.

SetSubject should probably raise NotImplemented, not InvalidArgument, on protocols without a concept of subjects? (I assume that's what you intended InvalidArgument to mean?)

NotCapable isn't the right error for insufficient privileges, you should use PermissionDenied. NotCapable is about e.g. Jingle capabilities.

RoomID seems unfortunately-named for the human readable version - I'd suggest RoomName so that we could add RoomID to this interface later as the UUID (if Target* disappears in favour of ContactID and RoomID in future...)

(In reply to comment #28)
> 14:08 < smcv> jonnylamb: I think you're probably right, we should bring back 
>               Chan.I.Subject

Done.

> If a protocol has the concept of a room subject, but has a distinction between
> "no subject" and "subject = ''", or "subject not known yet" and "subject = ''",
> would Present be set at all times, or only when a subject is known, or only
> when a non-empty subject is known? My guess would be that Present would be set
> at all times (so it MUST NOT change), 

Yeah that's how I saw it when I added it. It merely states that this protocol supports room subjects. I've added more documentation to this effect.

> Does Can_Set mean "the protocol allows me to try to set the subject" or "I am
> sufficiently privileged to actually set the subject"? If the former, it MUST
> NOT change; if the latter (which I assume is what you meant), obviously it
> might change. However, you can't actually tell the latter reliably in XMPP. I
> researched some MUC implementations a while ago, and they don't all interpret
> the informational flag in the same way :-( You can get pretty close, as Gabble
> currently does for the Tp Properties.

Well I'm currently giving it exactly the same value as the RW flag for the "subject" Tp property in gabble, as you've seen. I'm not sure how to describe that though -- what is a better name than Can_Set then?

> SetSubject should probably raise NotImplemented, not InvalidArgument, on
> protocols without a concept of subjects? (I assume that's what you intended
> InvalidArgument to mean?)

Actually I thought in the case of protocols which don't support UTF-8 subjects? But I guess that's actually none for now? It's easy enough to add later I guess. I'll change that to NotImplemented.

> NotCapable isn't the right error for insufficient privileges, you should use
> PermissionDenied. NotCapable is about e.g. Jingle capabilities.

Ah yes good point, I forgot about that. Changed.

(In reply to comment #29)
> RoomID seems unfortunately-named for the human readable version - I'd suggest
> RoomName so that we could add RoomID to this interface later as the UUID (if
> Target* disappears in favour of ContactID and RoomID in future...)

Done.

So I'm basically reviewing Room and Subject from first principles.

What's the point of the Room_Subject_Present flag? Surely if the Subject interface is implemented, then the channel has a subject; and if not, it doesn't? Is this a remnant of Subject being part of Room? Or is it to support the distinction between “the room has no topic” and “I don't know what the room's topic is”? I would like to think we could avoid caring about the latter in the API.

I think Can_Set's docstring should be clarified to say that it refers to having permission to change the subject; and that if the implementation isn't sure, it SHOULD be set, citing the inadequacies of XEP-0045 in the rationale. I think the name is fine, the description is just misleading.

If we got rid of the Present flag, then I think the change notification for the flags could be a separate signal. (Actually I think we could use DBus.Properties.PropertyChanged for this interface.)

I can see an argument for having the ssx bit of the subject be a single property, though it might be clearer to have them be separate properties; but I feel like the flags should be separate. (Or we could even have a CanSetSubject: b property. Adding more properties isn't more expensive than adding new enum members.)

The preamble should not say “by Room channels”, it should say “by channels with the <tp:dbus-ref...>Room</> interface”.

> Clients SHOULD NOT assume success if no errors are raised by calling
> this method, but instead should listen to the SubjectChanged signal
> for change notification.

Why? I think the method should not return until it's either succeeded or failed; but clients SHOULD monitor the SubjectChanged signal in case the topic was modified/truncated by the server or whatever.

RoomName and Server should grow the immutable and requestable attributes.

Oh maybe also mention in the preambles that both interfaces could apply to (eg) calls.

(In reply to comment #31)
> What's the point of the Room_Subject_Present flag? Surely if the Subject
> interface is implemented, then the channel has a subject; and if not, it
> doesn't? Is this a remnant of Subject being part of Room?

Yes.

> I think Can_Set's docstring should be clarified to say that it refers to having
> permission to change the subject; and that if the implementation isn't sure, it
> SHOULD be set, citing the inadequacies of XEP-0045 in the rationale. I think
> the name is fine, the description is just misleading.

Done.

> If we got rid of the Present flag, then I think the change notification for the
> flags could be a separate signal. (Actually I think we could use
> DBus.Properties.PropertyChanged for this interface.)

Done.

> I can see an argument for having the ssx bit of the subject be a single
> property, though it might be clearer to have them be separate properties;

Done.

> but I feel like the flags should be separate. (Or we could even have a
> CanSetSubject: b property. Adding more properties isn't more expensive
> than adding new enum members.)

Done.

> The preamble should not say “by Room channels”, it should say “by channels 
> with the <tp:dbus-ref...>Room</> interface”.

Done.

> > Clients SHOULD NOT assume success if no errors are raised by calling
> > this method, but instead should listen to the SubjectChanged signal
> > for change notification.
> 
> Why? I think the method should not return until it's either succeeded or
> failed; but clients SHOULD monitor the SubjectChanged signal in case the topic
> was modified/truncated by the server or whatever.

Okay, done.

> RoomName and Server should grow the immutable and requestable attributes.

Done.

(In reply to comment #32)
> Oh maybe also mention in the preambles that both interfaces could apply to (eg)
> calls.

Done.

       <tp:docstring>
         Emitted when the <tp:member-ref>Subject</tp:member-ref>
-        property is changed.
+        property is changed, otherwise FALSE.
       </tp:docstring>

What is this I don't even. This signal gets removed later, so never mind!

     <property name="Subject" tp:name-for-bindings="Subject"
-              type="s" access="read">
+              type="s" access="read"
+              org.freedesktop.DBus.Property.EmitsChangedSignal="updates">

I'm pretty sure this is not how D-Bus annotations work.

http://cgit.collabora.com/git/user/wjt/telepathy-spec-wjt.git/commit/?h=a-bit-more-room

I've expanded the branch to add replacements for all the Telepathy Properties on Text, remove all the old Properties, and deprecate that interface.

http://people.freedesktop.org/~wjt/telepathy-spec-a_bit_more_room/spec/
http://cgit.collabora.com/git/user/wjt/telepathy-spec-wjt.git/log/?h=a-bit-more-room

This is motivated by the presence of two PropertiesChanged signals breaking one or other of Subject and Text's change notifications, courtesy of dbus-glib.

Sorry, please don't review that branch. I had second thoughts about it.

Branch updated. I think it's okay now. I'm gonna get back to implementing it in Gabble.

Some of your property descriptions in RoomConfig1 have <code>True</code> and some just have True. Of all the people who I would have thought this would annoy the most... ;-)

Also some of the properties have <tp:added ...> and some don't? 

Why not just merge CanUpdateConfiguration and MutableProperties so if MutableProperties == [] then CanUpdateConfiguration == FALSE?

(In reply to comment #38)
> Some of your property descriptions in RoomConfig1 have <code>True</code> and
> some just have True. Of all the people who I would have thought this would
> annoy the most... ;-)

Fixed.

> Also some of the properties have <tp:added ...> and some don't?

A relic from when those properties lived on Password. Fixed.

> Why not just merge CanUpdateConfiguration and MutableProperties so if
> MutableProperties == [] then CanUpdateConfiguration == FALSE?

The distinction is “it might be possible to change these properties, if you were an op” vs. “these are just facts about this protocol”. If you don't think the difference is important—given that we don't have any way to op or be opped at the moment…—I can merge them.

Hokay, I've updated this branch. New HTML at http://people.freedesktop.org/~wjt/telepathy-spec-a_bit_more_room/spec/ as always.

I added a ConfigurationRetrieved property, and I undrafted Room, Subject and RoomConfig (adding version numbers to the first two in the process).

My only objection is for Subject::Actor property, we also need ActorHandle. In tp-glib/qt4 we must have handle+id pair to create a TpContact object without async operation. Similar as I did for bug #38248.

Okay, let's do this. Merged to master with Xavier's point addressed.