Subject: [Simple] Simple chat: Christer's comments

From: "Miguel A. Garcia" <[email-masked]>
Date: Sat, 25 Feb 2012 10:57:46 +0100

I am now addressing comments that Christer did in his Gen-ART review. I 
want to thank Christer for sending these comments.

See his comments below and my inline reply.

> Minor issues:
>
> - The second paragraph of section 7.1 says that a NICKNAME request
> MUST contain a Use-Nickname header, but in the sixth paragraph the
> inclusion is a SHOULD.

That is not totally correct. The second paragraph says:

"The NICKNAME request MUST include a new Use-Nickname header"

whereas the sixth paragraph tries to say (but apparently failed) in which 
methods the Use-Nickname header could be included:

    The Use-Nickname header field carries a
    nickname string, and SHOULD be included in the NICKNAME requests.

I proposed to add "only" to clarify the paragraph:

    The Use-Nickname header field carries a
    nickname string and SHOULD only be included in NICKNAME requests.

>
> - It is not clearly indicated whether the Use-Nickname header is
> allowed for other methods than NICKNAME.

I think it should be clear now, see previous comment.

> - Section 8 does not specify whether there are SDP offer/answer
> considerations/restrictions associated with the new attribute. For
> example: -- Must the attribute tokens in an answer be a subset of the
> tokens in an offer? -- Can an SDP answer contain an attribute if the
> offer didn't? -- If a user sends a new SDP offer within a session, can
> the token values be modified? What does it mean if the attribute is
> not present in a new SDP offer?
>

Good point. I have reworded these paragraphs, let me know what you think:

    The 'chatroom' attribute merely indicates the capabilities supported
    and allowed by the local policy.  This attribute is not a negotiation
    subject to the SDP offer/answer model, but instead a declaration.
    Therefore, a 'chatroom' attribute included in an SDP answer does not
    need to be a subset of the 'chatroom' attribute included in its
    corresponding SDP offer.  It is also possible that an SDP answer
    contains a 'chatroom' attribute even if its corresponding SDP offer
    did not include it.

    On doing subsequent SDP offer/answer exchanges pertaining to the same
    session, the 'chatroom' attribute MAY be modified with respect an
    earlier SDP offer/answer exchange.  The new value of this attribute
    indicate the current support and local policy, meaning that some
    restrictions can apply now or might have been removed.

I have also added a normative reference to RFC 3264 in the above text and 
other parts of the draft where the SDP offer/answer model is mentioned.

>
> Nits/editorial comments:
>
> - Sometimes the document talks about "multi-party chat", "multi-party
> conference", "conference", and "chat room". Would it be possible to
> use more consistant terminology?
>

Yes.

Wherever possible, i.e., in most places, I am trying to use "chat room". 
There are places where is not possible, for example, when we refer to the 
conference event package, conference framework, conference focus, etc. 
But I guess this solves your concern.

> - Requirements
> -- REQ-4: Isn't this requirement already covered by
> REQ-3?

No, these are different. Req-3 claims for a mechanism for the recipient 
of a message to determine whether the receive message is private or regular.

Req-4 claims for the a mechanism to send private messages.

> -- REQ-6: Change "progress" to "duration" or "length".

Done.

>
> - There is no definition/reference for "roster".
>

Roster is a not a technical term, therefore, it is not subject to be 
define in this draft.

BR,

     Miguel

-- 
Miguel A. Garcia
+34-91-339-3608
Ericsson Spain

---

From: Christer Holmberg <[email-masked]>
Date: Sun, 26 Feb 2012 20:05:25 +0100

Hi,

>    nickname string and SHOULD only be included in NICKNAME requests.

Why not "MUST only"?

>    contains a 'chatroom' attribute even if its corresponding SDP offer

Would it be better to say that it is allowed to included a 'chatroom' attribute in an SDP answer, even if the associated offer did not contain one?

That way you actually describe answerer behavior, rather than just indicating that the offerer may receive the attribute in an answer.

>    restrictions can apply now or might have been removed.

It is good, but to be really clear I would also suggest the following sentence - where I'll let you add the end of the sentence :)

"If the 'chatroom' attribute is not included in a subsequent SDP offer/answer, it indicates that <insert-end-of-sentence>."

Yes.

> Req-4 claims for the a mechanism to send private messages.

Correct. My misstake.

> Roster is a not a technical term, therefore, it is not subject to be define in this draft.

Ok.

Regards,

Christer

---

From: "Miguel A. Garcia" <[email-masked]>
Date: Mon, 27 Feb 2012 09:26:18 +0100

Hi Christer, see some inline replies.

>>     nickname string and SHOULD only be included in NICKNAME requests.

Because I want to leave an open door for someone who has a very good 
reason to add this header to a method different than NICKNAME.

> That way you actually describe answerer behavior, rather than just indicating that the offerer may receive the attribute in an answer.

I have now replaced the last sentence of the proposed new text with this one:

         Consequently, an SDP answer MAY contain a 'chatroom'
	attribute even if its corresponding SDP offer did not include
	it.

> "If the 'chatroom' attribute is not included in a subsequent SDP offer/answer, it indicates that<insert-end-of-sentence>."

Ok, this is the new sentence:

         If the 'chatroom' attribute is not included in a
	subsequent SDP offer/answer, but is corresponding MSRP stream
	is still in place, it indicates that support for the
	procedures indicated in this document are disabled.

/Miguel

-- 
Miguel A. Garcia
+34-91-339-3608
Ericsson Spain

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 27 Feb 2012 09:42:06 +0100

Hi, 

>>> whereas the sixth paragraph tries to say (but apparently failed) in 
>>> which methods the Use-Nickname header could be included:
>>>
> Because I want to leave an open door for someone who has a very good reason to add this header to a method different than NICKNAME.

Fair enough, but if that is the intention then I don't think SHOULD fits either. Instead, I rather write that this specification only defines the usage of the header for NICKNAME, and if someone wants to use it with another method the usage must be defined in a dedicated spec.

Ok.

>	procedures indicated in this document are disabled.

Good.

Regards,

Christer

---

From: "Miguel A. Garcia" <[email-masked]>
Date: Mon, 27 Feb 2012 10:44:23 +0100

On 27/02/2012 9:42, Christer Holmberg wrote:
> Fair enough, but if that is the intention then I don't think SHOULD
> fits either. Instead, I rather write that this specification only
> defines the usage of the header for NICKNAME, and if someone wants to
> use it with another method the usage must be defined in a dedicated
> spec.

What about this text?

   As indicated earlier, this specification defines a new MSRP header
   field: "Use-Nickname".  The Use-Nickname header field carries a
   nickname string.  This specification defines the usage of the
   Use-Nickname header field in NICKNAME requests.  If need arises,
   usages of the Use-Nickname header field in other MSRP methods should
   be specified separately.

BR,

     Miguel

-- 
Miguel A. Garcia
+34-91-339-3608
Ericsson Spain

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 27 Feb 2012 10:55:32 +0100

Hi, 

>   usages of the Use-Nickname header field in other MSRP methods should

Looks good.

Regards,

Christer