qxmpp icon indicating copy to clipboard operation
qxmpp copied to clipboard

room->setSubject("") does not work

Open Thomas-Lerman opened this issue 9 years ago • 2 comments

Because of the following code:

void QXmppMessage::toXml(QXmlStreamWriter *xmlWriter) const
{
    xmlWriter->writeStartElement("message");
    helperToXmlAddAttribute(xmlWriter, "xml:lang", lang());
    helperToXmlAddAttribute(xmlWriter, "id", id());
    helperToXmlAddAttribute(xmlWriter, "to", to());
    helperToXmlAddAttribute(xmlWriter, "from", from());
    helperToXmlAddAttribute(xmlWriter, "type", message_types[d->type]);
    if (!d->subject.isEmpty())
        helperToXmlAddTextElement(xmlWriter, "subject", d->subject);
   ...
}

the XML ends up being:

<message type="groupchat" to="[email protected]"/>

with a reply of:

<message from="[email protected]/admin" type="groupchat" to="admin@think-thomas/QXmpp"/>

which does not do anything with the subject.


If the following line is not executed:

    if (!d->subject.isEmpty())

the XML ends up being:

<message type="groupchat" to="[email protected]">
  <subject/>
</message>

which clears out the subject as shown in the following reply:

<message from="[email protected]/admin" type="groupchat" to="admin@think-thomas/QXmpp">
  <subject/>
</message>

However, as you see in the following code:

void QXmppMucRoom::_q_messageReceived(const QXmppMessage &message)
{
    if (QXmppUtils::jidToBareJid(message.from())!= d->jid)
        return;

    // handle message subject
    const QString subject = message.subject();
    if (!subject.isEmpty()) {
        d->subject = subject;
        emit subjectChanged(subject);
    }

    emit messageReceived(message);
}

it also has a similar if-statement and therefore does not update the subject nor emit the signal. Again, not executing the if-statement seems to fix it.

I am sure that just getting rid of these if-statements is not the correct thing to do.

Thomas-Lerman avatar Oct 11 '15 07:10 Thomas-Lerman

OK, i see two possible fixes:

  • the ifs could be changed from isEmpty to isNull
  • we add some other property like "hasSubject" to explicitly convey whether a message has a subject

In any case a fix needs to come with unit tests

jlaine avatar Oct 11 '15 08:10 jlaine

I was thinking about the latter, but either will work.

Thomas-Lerman avatar Oct 11 '15 16:10 Thomas-Lerman